Skip to content

Commit

Permalink
Implement new update API for writing systems (#1240)
Browse files Browse the repository at this point in the history
* Implement UpdateWritingSystem(before, after)

* mark up string diff as allowing null strings

---------

Co-authored-by: Kevin Hahn <[email protected]>
  • Loading branch information
rmunn and hahn-kev authored Nov 28, 2024
1 parent 18530d4 commit 7d002cf
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 11 deletions.
36 changes: 34 additions & 2 deletions backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ private WritingSystem FromLcmWritingSystem(CoreWritingSystemDefinition ws, int i
};
}

public Task<WritingSystem> GetWritingSystem(WritingSystemId id, WritingSystemType type)
{
throw new NotImplementedException();
}

internal void CompleteExemplars(WritingSystems writingSystems)
{
var wsExemplars = writingSystems.Vernacular.Concat(writingSystems.Analysis)
Expand Down Expand Up @@ -186,9 +191,36 @@ public Task<WritingSystem> CreateWritingSystem(WritingSystemType type, WritingSy
return Task.FromResult(FromLcmWritingSystem(ws, index, type));
}

public Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput<WritingSystem> update)
public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput<WritingSystem> 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 () =>

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Build FwHeadless / publish-fw-headless

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite and run tests

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Mac

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Mac

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Windows

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 202 in backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Windows

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
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<WritingSystem> 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<PartOfSpeech> GetPartsOfSpeech()
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
6 changes: 6 additions & 0 deletions backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id,
}).First(w => w.WsId == id);
}

public Task<WritingSystem> UpdateWritingSystem(WritingSystem before, WritingSystem after)
{
DryRunRecords.Add(new DryRunRecord(nameof(UpdateEntry), $"Update {after.Type} writing system {after.WsId}"));
return Task.FromResult(after);
}

public IAsyncEnumerable<PartOfSpeech> GetPartsOfSpeech()
{
return api.GetPartsOfSpeech();
Expand Down
6 changes: 6 additions & 0 deletions backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,12 @@ public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, Writing
return await dataModel.GetLatest<WritingSystem>(ws.Id) ?? throw new NullReferenceException();
}

public async Task<WritingSystem> 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<WritingSystem?> GetWritingSystem(WritingSystemId id, WritingSystemType type)
Expand Down
3 changes: 2 additions & 1 deletion backend/FwLite/MiniLcm/IMiniLcmWriteApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public interface IMiniLcmWriteApi
Task<WritingSystem> UpdateWritingSystem(WritingSystemId id,
WritingSystemType type,
UpdateObjectInput<WritingSystem> update);

Task<WritingSystem> 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<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech);
Expand Down
8 changes: 4 additions & 4 deletions backend/FwLite/MiniLcm/Models/WritingSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down
4 changes: 2 additions & 2 deletions backend/FwLite/MiniLcm/SyncHelpers/PartOfSpeechSync.cs
Original file line number Diff line number Diff line change
@@ -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<int> Sync(PartOfSpeech[] currentPartsOfSpeech,
Expand Down
4 changes: 2 additions & 2 deletions backend/FwLite/MiniLcm/SyncHelpers/SemanticDomainSync.cs
Original file line number Diff line number Diff line change
@@ -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<int> Sync(SemanticDomain[] currentSemanticDomains,
Expand Down
16 changes: 16 additions & 0 deletions backend/FwLite/MiniLcm/SyncHelpers/SimpleStringDiff.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
using SystemTextJsonPatch.Operations;

namespace MiniLcm.SyncHelpers;

public static class SimpleStringDiff
{
public static IEnumerable<Operation<T>> GetStringDiff<T>(string path,
string? before,
string? after) where T : class
{
if (before == after) yield break;
if (after is null) yield return new Operation<T>("remove", $"/{path}", null);
else if (before is null) yield return new Operation<T>("add", $"/{path}", null);
else yield return new Operation<T>("replace", $"/{path}", null, after);
}
}
61 changes: 61 additions & 0 deletions backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
using MiniLcm.Models;
using SystemTextJsonPatch;

namespace MiniLcm.SyncHelpers;

public static class WritingSystemSync
{
public static async Task<int> 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) =>

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Build FwHeadless / publish-fw-headless

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite and run tests

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Mac

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Mac

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Linux

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Windows

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.

Check warning on line 21 in backend/FwLite/MiniLcm/SyncHelpers/WritingSystemSync.cs

View workflow job for this annotation

GitHub Actions / Publish FW Lite app for Windows

This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
{
// 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<int> 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<WritingSystem>? WritingSystemDiffToUpdate(WritingSystem previousWritingSystem, WritingSystem currentWritingSystem)
{
JsonPatchDocument<WritingSystem> 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<WritingSystem>(nameof(WritingSystem.Name),
previousWritingSystem.Name,
currentWritingSystem.Name));
patchDocument.Operations.AddRange(SimpleStringDiff.GetStringDiff<WritingSystem>(nameof(WritingSystem.Abbreviation),
previousWritingSystem.Abbreviation,
currentWritingSystem.Abbreviation));
patchDocument.Operations.AddRange(SimpleStringDiff.GetStringDiff<WritingSystem>(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<WritingSystem>(patchDocument);
}
}

0 comments on commit 7d002cf

Please sign in to comment.