From 7d002cfdbb7b9b7fea470a04b8718ebfe9ebc0b1 Mon Sep 17 00:00:00 2001 From: Robin Munn Date: Thu, 28 Nov 2024 13:41:52 +0700 Subject: [PATCH] Implement new update API for writing systems (#1240) * Implement UpdateWritingSystem(before, after) * mark up string diff as allowing null strings --------- Co-authored-by: Kevin Hahn --- .../Api/FwDataMiniLcmApi.cs | 36 +++++++++- .../UpdateProxy/UpdateWritingSystemProxy.cs | 69 +++++++++++++++++++ .../FwLiteProjectSync/DryRunMiniLcmApi.cs | 6 ++ backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 6 ++ backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs | 3 +- .../FwLite/MiniLcm/Models/WritingSystem.cs | 8 +-- .../MiniLcm/SyncHelpers/PartOfSpeechSync.cs | 4 +- .../MiniLcm/SyncHelpers/SemanticDomainSync.cs | 4 +- .../MiniLcm/SyncHelpers/SimpleStringDiff.cs | 16 +++++ .../MiniLcm/SyncHelpers/WritingSystemSync.cs | 61 ++++++++++++++++ 10 files changed, 202 insertions(+), 11 deletions(-) create mode 100644 backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateWritingSystemProxy.cs create mode 100644 backend/FwLite/MiniLcm/SyncHelpers/SimpleStringDiff.cs create mode 100644 backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs index 599e4a900..058f1d0ba 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs @@ -135,6 +135,11 @@ private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, int i }; } + public Task GetWritingSystem(WritingSystemId id, WritingSystemType type) + { + throw new NotImplementedException(); + } + internal void CompleteExemplars(WritingSystems writingSystems) { var wsExemplars = writingSystems.Vernacular.Concat(writingSystems.Analysis) @@ -186,9 +191,36 @@ public Task CreateWritingSystem(WritingSystemType type, WritingSy return Task.FromResult(FromLcmWritingSystem(ws, index, type)); } - public Task UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput update) + public async Task UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput update) { - throw new NotImplementedException(); + if (!Cache.ServiceLocator.WritingSystemManager.TryGet(id.Code, out var lcmWritingSystem)) + { + throw new InvalidOperationException($"Writing system {id.Code} not found"); + } + await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem", + "Revert WritingSystem", + async () => + { + var updateProxy = new UpdateWritingSystemProxy(lcmWritingSystem, this) + { + Id = Guid.Empty, + Type = type, + }; + update.Apply(updateProxy); + updateProxy.CommitUpdate(Cache); + }); + return await GetWritingSystem(id, type); + } + + public async Task UpdateWritingSystem(WritingSystem before, WritingSystem after) + { + await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem", + "Revert WritingSystem", + async () => + { + await WritingSystemSync.Sync(after, before, this); + }); + return await GetWritingSystem(after.WsId, after.Type) ?? throw new NullReferenceException($"unable to find {after.Type} writing system with id {after.WsId}"); } public IAsyncEnumerable GetPartsOfSpeech() diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateWritingSystemProxy.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateWritingSystemProxy.cs new file mode 100644 index 000000000..eaf849fcb --- /dev/null +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/UpdateProxy/UpdateWritingSystemProxy.cs @@ -0,0 +1,69 @@ +using System.Diagnostics.CodeAnalysis; +using MiniLcm.Models; +using SIL.LCModel; +using SIL.LCModel.Core.WritingSystems; +using SIL.LCModel.DomainServices; +using SIL.WritingSystems; + +namespace FwDataMiniLcmBridge.Api.UpdateProxy; + +public record UpdateWritingSystemProxy : WritingSystem +{ + private readonly CoreWritingSystemDefinition _origLcmWritingSystem; + private readonly CoreWritingSystemDefinition _workingLcmWritingSystem; + private readonly FwDataMiniLcmApi _lexboxLcmApi; + + [SetsRequiredMembers] + public UpdateWritingSystemProxy(CoreWritingSystemDefinition lcmWritingSystem, FwDataMiniLcmApi lexboxLcmApi) + { + _origLcmWritingSystem = lcmWritingSystem; + _workingLcmWritingSystem = new CoreWritingSystemDefinition(lcmWritingSystem, cloneId: true); + base.Abbreviation = Abbreviation = _origLcmWritingSystem.Abbreviation ?? ""; + base.Name = Name = _origLcmWritingSystem.LanguageName ?? ""; + base.Font = Font = _origLcmWritingSystem.DefaultFontName ?? ""; + _lexboxLcmApi = lexboxLcmApi; + } + + public void CommitUpdate(LcmCache cache) + { + if (_workingLcmWritingSystem.Id == _origLcmWritingSystem.Id) + { + cache.ServiceLocator.WritingSystemManager.Set(_workingLcmWritingSystem); + } + else + { + // Changing the ID of a writing system requires LCM to do a lot of work, so only go through that process if absolutely required + WritingSystemServices.MergeWritingSystems(cache, _workingLcmWritingSystem, _origLcmWritingSystem); + } + } + + public override required WritingSystemId WsId + { + get => _workingLcmWritingSystem.Id; + set => _workingLcmWritingSystem.Id = value; + } + + public override required string Name + { + get => _workingLcmWritingSystem.LanguageName; + set { } // Silently do nothing; name should be derived from WsId at all times, so if the name should change then so should the WsId + } + + public override required string Abbreviation + { + get => _workingLcmWritingSystem.Abbreviation; + set => _workingLcmWritingSystem.Abbreviation = value; + } + + public override required string Font + { + get => _workingLcmWritingSystem.DefaultFontName; + set + { + if (value != _workingLcmWritingSystem.DefaultFontName) + { + _workingLcmWritingSystem.DefaultFont = new FontDefinition(value); + } + } + } +} diff --git a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs index 15c17a387..11635dfa3 100644 --- a/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs +++ b/backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs @@ -34,6 +34,12 @@ public async Task UpdateWritingSystem(WritingSystemId id, }).First(w => w.WsId == id); } + public Task UpdateWritingSystem(WritingSystem before, WritingSystem after) + { + DryRunRecords.Add(new DryRunRecord(nameof(UpdateEntry), $"Update {after.Type} writing system {after.WsId}")); + return Task.FromResult(after); + } + public IAsyncEnumerable GetPartsOfSpeech() { return api.GetPartsOfSpeech(); diff --git a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs index 28cc4048e..ce2bff06f 100644 --- a/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs +++ b/backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs @@ -57,6 +57,12 @@ public async Task UpdateWritingSystem(WritingSystemId id, Writing return await dataModel.GetLatest(ws.Id) ?? throw new NullReferenceException(); } + public async Task UpdateWritingSystem(WritingSystem before, WritingSystem after) + { + await WritingSystemSync.Sync(after, before, this); + return await GetWritingSystem(after.WsId, after.Type) ?? throw new NullReferenceException("unable to find writing system with id " + after.WsId); + } + private WritingSystem? _defaultVernacularWs; private WritingSystem? _defaultAnalysisWs; private async Task GetWritingSystem(WritingSystemId id, WritingSystemType type) diff --git a/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs b/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs index 4685f6d37..6608c6335 100644 --- a/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs +++ b/backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs @@ -11,7 +11,8 @@ public interface IMiniLcmWriteApi Task UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput update); - + Task UpdateWritingSystem(WritingSystem before, WritingSystem after); + // Note there's no Task DeleteWritingSystem(Guid id) because deleting writing systems needs careful consideration, as it can cause a massive cascade of data deletion #region PartOfSpeech Task CreatePartOfSpeech(PartOfSpeech partOfSpeech); diff --git a/backend/FwLite/MiniLcm/Models/WritingSystem.cs b/backend/FwLite/MiniLcm/Models/WritingSystem.cs index ed96bcddc..e23627cd7 100644 --- a/backend/FwLite/MiniLcm/Models/WritingSystem.cs +++ b/backend/FwLite/MiniLcm/Models/WritingSystem.cs @@ -3,10 +3,10 @@ public record WritingSystem: IObjectWithId { public required Guid Id { get; set; } - public required WritingSystemId WsId { get; set; } - public required string Name { get; set; } - public required string Abbreviation { get; set; } - public required string Font { get; set; } + public virtual required WritingSystemId WsId { get; set; } + public virtual required string Name { get; set; } + public virtual required string Abbreviation { get; set; } + public virtual required string Font { get; set; } public DateTimeOffset? DeletedAt { get; set; } public required WritingSystemType Type { get; set; } diff --git a/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs index a37e88cc8..f5bff17e0 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs @@ -1,8 +1,8 @@ -using MiniLcm; using MiniLcm.Models; -using MiniLcm.SyncHelpers; using SystemTextJsonPatch; +namespace MiniLcm.SyncHelpers; + public static class PartOfSpeechSync { public static async Task Sync(PartOfSpeech[] currentPartsOfSpeech, diff --git a/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs index 1219db9aa..d1e150d95 100644 --- a/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs +++ b/backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs @@ -1,8 +1,8 @@ -using MiniLcm; using MiniLcm.Models; -using MiniLcm.SyncHelpers; using SystemTextJsonPatch; +namespace MiniLcm.SyncHelpers; + public static class SemanticDomainSync { public static async Task Sync(SemanticDomain[] currentSemanticDomains, diff --git a/backend/FwLite/MiniLcm/SyncHelpers/SimpleStringDiff.cs b/backend/FwLite/MiniLcm/SyncHelpers/SimpleStringDiff.cs new file mode 100644 index 000000000..37f9e7870 --- /dev/null +++ b/backend/FwLite/MiniLcm/SyncHelpers/SimpleStringDiff.cs @@ -0,0 +1,16 @@ +using SystemTextJsonPatch.Operations; + +namespace MiniLcm.SyncHelpers; + +public static class SimpleStringDiff +{ + public static IEnumerable> GetStringDiff(string path, + string? before, + string? after) where T : class + { + if (before == after) yield break; + if (after is null) yield return new Operation("remove", $"/{path}", null); + else if (before is null) yield return new Operation("add", $"/{path}", null); + else yield return new Operation("replace", $"/{path}", null, after); + } +} diff --git a/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs b/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs new file mode 100644 index 000000000..1930059ce --- /dev/null +++ b/backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs @@ -0,0 +1,61 @@ +using MiniLcm.Models; +using SystemTextJsonPatch; + +namespace MiniLcm.SyncHelpers; + +public static class WritingSystemSync +{ + public static async Task Sync(WritingSystem[] currentWritingSystems, + WritingSystem[] previousWritingSystems, + IMiniLcmApi api) + { + return await DiffCollection.Diff(api, + 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); + }); + } + + public static async Task Sync(WritingSystem afterWs, WritingSystem beforeWs, IMiniLcmApi api) + { + var updateObjectInput = WritingSystemDiffToUpdate(beforeWs, afterWs); + if (updateObjectInput is not null) await api.UpdateWritingSystem(afterWs.WsId, afterWs.Type, updateObjectInput); + return updateObjectInput is null ? 0 : 1; + } + + public static UpdateObjectInput? WritingSystemDiffToUpdate(WritingSystem previousWritingSystem, WritingSystem currentWritingSystem) + { + JsonPatchDocument patchDocument = new(); + if (previousWritingSystem.WsId != currentWritingSystem.WsId) + { + // TODO: Throw? Or silently ignore? + throw new InvalidOperationException($"Tried to change immutable WsId from {previousWritingSystem.WsId} to {currentWritingSystem.WsId}"); + } + patchDocument.Operations.AddRange(SimpleStringDiff.GetStringDiff(nameof(WritingSystem.Name), + previousWritingSystem.Name, + currentWritingSystem.Name)); + patchDocument.Operations.AddRange(SimpleStringDiff.GetStringDiff(nameof(WritingSystem.Abbreviation), + previousWritingSystem.Abbreviation, + currentWritingSystem.Abbreviation)); + patchDocument.Operations.AddRange(SimpleStringDiff.GetStringDiff(nameof(WritingSystem.Font), + previousWritingSystem.Font, + currentWritingSystem.Font)); + // TODO: Exemplars, Order, and do we need DeletedAt? + if (patchDocument.Operations.Count == 0) return null; + return new UpdateObjectInput(patchDocument); + } +}