From 165a44a39dd33871473548694bb81a6b0ef0c2f8 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 11 Dec 2024 14:15:41 +0700 Subject: [PATCH 1/4] Chore/fix serilization error add entry component change (#1310) * remove headword parameters which are no longer used * write tests to ensure we can round trip serialize all crdt changes * fix serialization bugs * use faker to generate changes --- .../Changes/ChangeSerializationTests.cs | 82 +++++++++++++++++++ .../FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs | 2 + .../Changes/AddSemanticDomainChange.cs | 4 +- .../Entries/AddEntryComponentChange.cs | 4 - .../Changes/RemoveSemanticDomainChange.cs | 4 +- .../Changes/ReplaceSemanticDomainChange.cs | 4 +- backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 14 ++++ 7 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 backend/FwLite/LcmCrdt.Tests/Changes/ChangeSerializationTests.cs diff --git a/backend/FwLite/LcmCrdt.Tests/Changes/ChangeSerializationTests.cs b/backend/FwLite/LcmCrdt.Tests/Changes/ChangeSerializationTests.cs new file mode 100644 index 000000000..287c32950 --- /dev/null +++ b/backend/FwLite/LcmCrdt.Tests/Changes/ChangeSerializationTests.cs @@ -0,0 +1,82 @@ +using System.Reflection; +using System.Text.Json; +using FluentAssertions.Execution; +using LcmCrdt.Changes; +using LcmCrdt.Changes.Entries; +using MiniLcm.Tests.AutoFakerHelpers; +using SIL.Harmony.Changes; +using Soenneker.Utils.AutoBogus; +using SystemTextJsonPatch; + +namespace LcmCrdt.Tests.Changes; + +public class ChangeSerializationTests +{ + private static readonly AutoFaker Faker = new() + { + Config = + { + Overrides = [new WritingSystemIdOverride()] + } + }; + + public static IEnumerable Changes() + { + foreach (var type in LcmCrdtKernel.AllChangeTypes()) + { + //can't generate this type because there's no public constructor, so its specified below + if (type == typeof(SetComplexFormComponentChange)) continue; + + object change; + if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(JsonPatchChange<>)) + { + change = PatchMethod.MakeGenericMethod(type.GenericTypeArguments[0]).Invoke(null, null)!; + } + else + { + change = Faker.Generate(type); + } + change.Should().NotBeNull($"change type {type.Name} should have been generated"); + yield return [change]; + } + yield return [SetComplexFormComponentChange.NewComplexForm(Guid.NewGuid(), Guid.NewGuid())]; + yield return [SetComplexFormComponentChange.NewComponent(Guid.NewGuid(), Guid.NewGuid())]; + yield return [SetComplexFormComponentChange.NewComponentSense(Guid.NewGuid(), Guid.NewGuid(), Guid.NewGuid())]; + } + + private static readonly MethodInfo PatchMethod = new Func(Patch).Method.GetGenericMethodDefinition(); + + private static IChange Patch() where T : class + { + return new JsonPatchChange(Guid.NewGuid(), new JsonPatchDocument()); + } + + [Theory] + [MemberData(nameof(Changes))] + public void CanRoundTripChanges(IChange change) + { + var config = new CrdtConfig(); + LcmCrdtKernel.ConfigureCrdt(config); + //commit id is not serialized + change.CommitId = Guid.Empty; + var type = change.GetType(); + var json = JsonSerializer.Serialize(change, config.JsonSerializerOptions); + var newChange = JsonSerializer.Deserialize(json, type, config.JsonSerializerOptions); + newChange.Should().BeEquivalentTo(change); + } + + [Fact] + public void ChangesIncludesAllValidChangeTypes() + { + var allChangeTypes = LcmCrdtKernel.AllChangeTypes(); + allChangeTypes.Should().NotBeEmpty(); + var testedTypes = Changes().Select(c => c[0].GetType()).ToArray(); + using (new AssertionScope()) + { + foreach (var allChangeType in allChangeTypes) + { + testedTypes.Should().Contain(allChangeType); + } + } + } +} diff --git a/backend/FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs b/backend/FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs index 1ce5a0321..dcc249a14 100644 --- a/backend/FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs +++ b/backend/FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using MiniLcm; using MiniLcm.Models; using Xunit.Abstractions; @@ -17,6 +18,7 @@ public class MiniLcmApiFixture : IAsyncLifetime private LcmCrdtDbContext? _crdtDbContext; public CrdtMiniLcmApi Api => (CrdtMiniLcmApi)_services.ServiceProvider.GetRequiredService(); public DataModel DataModel => _services.ServiceProvider.GetRequiredService(); + public CrdtConfig CrdtConfig => _services.ServiceProvider.GetRequiredService>().Value; public MiniLcmApiFixture() { diff --git a/backend/FwLite/LcmCrdt/Changes/AddSemanticDomainChange.cs b/backend/FwLite/LcmCrdt/Changes/AddSemanticDomainChange.cs index 42a8c3a7c..e47f621a4 100644 --- a/backend/FwLite/LcmCrdt/Changes/AddSemanticDomainChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/AddSemanticDomainChange.cs @@ -3,8 +3,8 @@ namespace LcmCrdt.Changes; -public class AddSemanticDomainChange(SemanticDomain semanticDomain, Guid senseId) - : EditChange(senseId), ISelfNamedType +public class AddSemanticDomainChange(SemanticDomain semanticDomain, Guid entityId) + : EditChange(entityId), ISelfNamedType { public SemanticDomain SemanticDomain { get; } = semanticDomain; diff --git a/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs b/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs index f93630058..f12bb9d85 100644 --- a/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/Entries/AddEntryComponentChange.cs @@ -16,9 +16,7 @@ public class AddEntryComponentChange : CreateChange, ISelf [JsonConstructor] public AddEntryComponentChange(Guid entityId, Guid complexFormEntryId, - string? complexFormHeadword, Guid componentEntryId, - string? componentHeadword, Guid? componentSenseId = null) : base(entityId) { ComplexFormEntryId = complexFormEntryId; @@ -28,9 +26,7 @@ public AddEntryComponentChange(Guid entityId, public AddEntryComponentChange(ComplexFormComponent component) : this(component.Id == default ? Guid.NewGuid() : component.Id, component.ComplexFormEntryId, - component.ComplexFormHeadword, component.ComponentEntryId, - component.ComponentHeadword, component.ComponentSenseId) { } diff --git a/backend/FwLite/LcmCrdt/Changes/RemoveSemanticDomainChange.cs b/backend/FwLite/LcmCrdt/Changes/RemoveSemanticDomainChange.cs index 80959cdd3..544c36d68 100644 --- a/backend/FwLite/LcmCrdt/Changes/RemoveSemanticDomainChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/RemoveSemanticDomainChange.cs @@ -3,8 +3,8 @@ namespace LcmCrdt.Changes; -public class RemoveSemanticDomainChange(Guid semanticDomainId, Guid senseId) - : EditChange(senseId), ISelfNamedType +public class RemoveSemanticDomainChange(Guid semanticDomainId, Guid entityId) + : EditChange(entityId), ISelfNamedType { public Guid SemanticDomainId { get; } = semanticDomainId; diff --git a/backend/FwLite/LcmCrdt/Changes/ReplaceSemanticDomainChange.cs b/backend/FwLite/LcmCrdt/Changes/ReplaceSemanticDomainChange.cs index f87e90afc..429bdc71b 100644 --- a/backend/FwLite/LcmCrdt/Changes/ReplaceSemanticDomainChange.cs +++ b/backend/FwLite/LcmCrdt/Changes/ReplaceSemanticDomainChange.cs @@ -3,8 +3,8 @@ namespace LcmCrdt.Changes; -public class ReplaceSemanticDomainChange(Guid oldSemanticDomainId, SemanticDomain semanticDomain, Guid senseId) - : EditChange(senseId), ISelfNamedType +public class ReplaceSemanticDomainChange(Guid oldSemanticDomainId, SemanticDomain semanticDomain, Guid entityId) + : EditChange(entityId), ISelfNamedType { public Guid OldSemanticDomainId { get; } = oldSemanticDomainId; public SemanticDomain SemanticDomain { get; } = semanticDomain; diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 08557d6bc..9b1495579 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -1,5 +1,7 @@ using System.Linq.Expressions; +using System.Reflection; using System.Text.Json; +using System.Text.Json.Serialization.Metadata; using SIL.Harmony; using SIL.Harmony.Core; using SIL.Harmony.Changes; @@ -191,6 +193,18 @@ public static void ConfigureCrdt(CrdtConfig config) .Add(); } + public static Type[] AllChangeTypes() + { + var crdtConfig = new CrdtConfig(); + ConfigureCrdt(crdtConfig); + + + var list = typeof(ChangeTypeListBuilder).GetProperty("Types", BindingFlags.Instance | BindingFlags.NonPublic) + ?.GetValue(crdtConfig.ChangeTypeListBuilder) as List; + return list?.Select(t => t.DerivedType).ToArray() ?? []; + } + + public static Task OpenCrdtProject(this IServiceProvider services, CrdtProject project) { //this method must not be async, otherwise Setting the project scope will not work as expected. From 4525771941b73c53d13c7f8524156d4afc67f7a5 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Wed, 11 Dec 2024 17:21:57 +0700 Subject: [PATCH 2/4] allow anonymous access to the ShouldUpdate api --- backend/LexBoxApi/Controllers/FwLiteReleaseController.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/LexBoxApi/Controllers/FwLiteReleaseController.cs b/backend/LexBoxApi/Controllers/FwLiteReleaseController.cs index 3f1c2ec09..24338c781 100644 --- a/backend/LexBoxApi/Controllers/FwLiteReleaseController.cs +++ b/backend/LexBoxApi/Controllers/FwLiteReleaseController.cs @@ -48,6 +48,7 @@ public async ValueTask> LatestRelease(string? appVer } [HttpGet("should-update")] + [AllowAnonymous] public async Task> ShouldUpdate([FromQuery] string appVersion) { using var activity = LexBoxActivitySource.Get().StartActivity(); From 4faa7a3a2e8a4f31119aa2124e29bd1db0d4cb32 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Thu, 12 Dec 2024 13:28:20 +0700 Subject: [PATCH 3/4] update to latest harmony release --- backend/harmony | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/harmony b/backend/harmony index c13987d13..f400e261a 160000 --- a/backend/harmony +++ b/backend/harmony @@ -1 +1 @@ -Subproject commit c13987d13f7fa4c37e0ebdd28b04e42a31df7e4c +Subproject commit f400e261a8309e4d74f8a47b1d4c1e9c3c998aff From f2101d5c0f1bd72302f63ca20896fd5db253a759 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 13 Dec 2024 09:56:56 +0700 Subject: [PATCH 4/4] fix auto updating package in use (#1317) * use api which supports deferring the update while the app is in use * trim info version to be exactly what we want instead of including the full commit hash --- .github/workflows/fw-lite.yaml | 6 ++---- .../FwDataMiniLcmBridge.csproj | 3 +-- backend/FwLite/FwLiteDesktop/AppVersion.cs | 15 +++++++++++++-- .../FwLite/FwLiteDesktop/FwLiteDesktop.csproj | 6 ++---- .../Platforms/Windows/AppUpdateService.cs | 16 +++++++++++++--- .../FwLiteProjectSync/FwLiteProjectSync.csproj | 1 - backend/FwLite/LcmCrdt/LcmCrdt.csproj | 1 - backend/FwLite/LocalWebApp/LocalWebApp.csproj | 1 - backend/FwLite/MiniLcm/MiniLcm.csproj | 1 - 9 files changed, 31 insertions(+), 19 deletions(-) diff --git a/.github/workflows/fw-lite.yaml b/.github/workflows/fw-lite.yaml index 5b3c2ccc9..28b3252d5 100644 --- a/.github/workflows/fw-lite.yaml +++ b/.github/workflows/fw-lite.yaml @@ -172,16 +172,14 @@ jobs: - name: Publish Windows MAUI portable app working-directory: backend/FwLite/FwLiteDesktop run: | - dotnet publish -r win-x64 --artifacts-path ../artifacts -p:WindowsPackageType=None -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} - dotnet publish -r win-arm64 --artifacts-path ../artifacts -p:WindowsPackageType=None -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} + dotnet publish -r win-x64 --artifacts-path ../artifacts -p:WindowsPackageType=None -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} -p:InformationalVersion=${{ needs.build-and-test.outputs.version }} mkdir -p ../artifacts/sign/portable cp -r ../artifacts/publish/FwLiteDesktop/* ../artifacts/sign/portable/ - name: Publish Windows MAUI msix app working-directory: backend/FwLite/FwLiteDesktop run: | - dotnet publish -r win-x64 --artifacts-path ../artifacts -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} - dotnet publish -r win-arm64 --artifacts-path ../artifacts -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} + dotnet publish -r win-x64 --artifacts-path ../artifacts -p:ApplicationDisplayVersion=${{ needs.build-and-test.outputs.semver-version }} -p:InformationalVersion=${{ needs.build-and-test.outputs.version }} mkdir -p ../artifacts/msix cp ../artifacts/bin/FwLiteDesktop/*/AppPackages/*/*.msix ../artifacts/msix/ diff --git a/backend/FwLite/FwDataMiniLcmBridge/FwDataMiniLcmBridge.csproj b/backend/FwLite/FwDataMiniLcmBridge/FwDataMiniLcmBridge.csproj index dd6ae3743..cd3878b24 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/FwDataMiniLcmBridge.csproj +++ b/backend/FwLite/FwDataMiniLcmBridge/FwDataMiniLcmBridge.csproj @@ -1,8 +1,7 @@  - $(ApplicationDisplayVersion) - $(ApplicationDisplayVersion) + $(ApplicationDisplayVersion) diff --git a/backend/FwLite/FwLiteDesktop/AppVersion.cs b/backend/FwLite/FwLiteDesktop/AppVersion.cs index 16f9f3421..b6ac0424c 100644 --- a/backend/FwLite/FwLiteDesktop/AppVersion.cs +++ b/backend/FwLite/FwLiteDesktop/AppVersion.cs @@ -4,6 +4,17 @@ namespace FwLiteDesktop; public class AppVersion { - public static readonly string Version = typeof(AppVersion).Assembly - .GetCustomAttribute()?.InformationalVersion ?? "dev"; + static AppVersion() + { + var infoVersion = typeof(AppVersion).Assembly + .GetCustomAttribute()?.InformationalVersion; + //info version may look like v2024-12-12-3073dd1c+3073dd1ce2ff5510f54a9411366f55c958b9ea45. We want to strip off everything after the +, so we can compare versions + if (infoVersion is not null && infoVersion.Contains('+')) + { + infoVersion = infoVersion[..infoVersion.IndexOf('+')]; + } + Version = infoVersion ?? "dev"; + } + + public static readonly string Version; } diff --git a/backend/FwLite/FwLiteDesktop/FwLiteDesktop.csproj b/backend/FwLite/FwLiteDesktop/FwLiteDesktop.csproj index d37d8b546..f3e46e10c 100644 --- a/backend/FwLite/FwLiteDesktop/FwLiteDesktop.csproj +++ b/backend/FwLite/FwLiteDesktop/FwLiteDesktop.csproj @@ -33,14 +33,12 @@ 1.0 1 - - $(ApplicationDisplayVersion) 11.0 13.1 21.0 - 10.0.17763.0 - 10.0.17763.0 + 10.0.19041.0 + 10.0.19041.0 6.5 diff --git a/backend/FwLite/FwLiteDesktop/Platforms/Windows/AppUpdateService.cs b/backend/FwLite/FwLiteDesktop/Platforms/Windows/AppUpdateService.cs index ca8b0ba2b..bf0e9721d 100644 --- a/backend/FwLite/FwLiteDesktop/Platforms/Windows/AppUpdateService.cs +++ b/backend/FwLite/FwLiteDesktop/Platforms/Windows/AppUpdateService.cs @@ -45,14 +45,24 @@ private async Task TryUpdate() private async Task ApplyUpdate(FwLiteRelease latestRelease) { - logger.LogInformation("New version available: {Version}", latestRelease.Version); + logger.LogInformation("New version available: {Version}, Current version: {CurrentVersion}", latestRelease.Version, AppVersion.Version); var packageManager = new PackageManager(); - var asyncOperation = packageManager.AddPackageAsync(new Uri(latestRelease.Url), [], DeploymentOptions.None); + var asyncOperation = packageManager.AddPackageByUriAsync(new Uri(latestRelease.Url), + new AddPackageOptions() + { + DeferRegistrationWhenPackagesAreInUse = true, + ForceUpdateFromAnyVersion = true + }); asyncOperation.Progress = (info, progressInfo) => { + if (progressInfo.state == DeploymentProgressState.Queued) + { + logger.LogInformation("Queued update"); + return; + } logger.LogInformation("Downloading update: {ProgressPercentage}%", progressInfo.percentage); }; - var result = await asyncOperation.AsTask(); + var result = await asyncOperation; if (!string.IsNullOrEmpty(result.ErrorText)) { logger.LogError(result.ExtendedErrorCode, "Failed to download update: {ErrorText}", result.ErrorText); diff --git a/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj b/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj index 8cbde01b8..2b3a8d940 100644 --- a/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj +++ b/backend/FwLite/FwLiteProjectSync/FwLiteProjectSync.csproj @@ -1,7 +1,6 @@  - $(ApplicationDisplayVersion) $(ApplicationDisplayVersion) diff --git a/backend/FwLite/LcmCrdt/LcmCrdt.csproj b/backend/FwLite/LcmCrdt/LcmCrdt.csproj index 66bd926ff..cbd1fa603 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdt.csproj +++ b/backend/FwLite/LcmCrdt/LcmCrdt.csproj @@ -1,7 +1,6 @@  - $(ApplicationDisplayVersion) $(ApplicationDisplayVersion) diff --git a/backend/FwLite/LocalWebApp/LocalWebApp.csproj b/backend/FwLite/LocalWebApp/LocalWebApp.csproj index 6b480b782..14c3dbd0a 100644 --- a/backend/FwLite/LocalWebApp/LocalWebApp.csproj +++ b/backend/FwLite/LocalWebApp/LocalWebApp.csproj @@ -5,7 +5,6 @@ true false true - $(ApplicationDisplayVersion) $(ApplicationDisplayVersion) diff --git a/backend/FwLite/MiniLcm/MiniLcm.csproj b/backend/FwLite/MiniLcm/MiniLcm.csproj index e806ed4ff..3a89c7e9e 100644 --- a/backend/FwLite/MiniLcm/MiniLcm.csproj +++ b/backend/FwLite/MiniLcm/MiniLcm.csproj @@ -1,7 +1,6 @@  - $(ApplicationDisplayVersion) $(ApplicationDisplayVersion)