From 77e409ef9543a74de1e5d264623ad13d901d569e Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 14 Jul 2025 11:11:48 +0700 Subject: [PATCH 1/7] add some required apis to harmony --- .../IRemoteResourceService.cs | 4 ++- src/SIL.Harmony/Db/CrdtRepository.cs | 5 ++++ .../Resource/CreateRemoteResourceChange.cs | 2 +- src/SIL.Harmony/Resource/HarmonyResource.cs | 6 +++- src/SIL.Harmony/ResourceService.cs | 30 +++++++++++++++++-- 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/SIL.Harmony.Core/IRemoteResourceService.cs b/src/SIL.Harmony.Core/IRemoteResourceService.cs index 22ebb06..3e4eefa 100644 --- a/src/SIL.Harmony.Core/IRemoteResourceService.cs +++ b/src/SIL.Harmony.Core/IRemoteResourceService.cs @@ -15,12 +15,14 @@ public interface IRemoteResourceService /// path defined by the CRDT config where the resource should be stored /// download result containing the path to the downloaded file, this is stored in the local db and not synced Task DownloadResource(string remoteId, string localResourceCachePath); + /// /// upload a resource to the remote server /// + /// id of the resource in the CRDT /// full path to the resource on the local machine /// an upload result with the remote id, the id will be stored and transmitted to other clients so they can also download the resource - Task UploadResource(string localPath); + Task UploadResource(Guid resourceId, string localPath); } public record DownloadResult(string LocalPath); diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 768613b..91d6ed3 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -390,6 +390,11 @@ public async Task AddLocalResource(LocalResource localResource) await _dbContext.SaveChangesAsync(); } + public async Task DeleteLocalResource(Guid id) + { + await _dbContext.Set().Where(r => r.Id == id).ExecuteDeleteAsync(); + } + public IAsyncEnumerable LocalResourcesByIds(IEnumerable resourceIds) { return _dbContext.Set().Where(r => resourceIds.Contains(r.Id)).AsAsyncEnumerable(); diff --git a/src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs b/src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs index 685ddb9..6840198 100644 --- a/src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs +++ b/src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs @@ -3,7 +3,7 @@ namespace SIL.Harmony.Resource; -public class CreateRemoteResourceChange(Guid resourceId, string remoteId) : CreateChange(resourceId), IPolyType +public class CreateRemoteResourceChange(Guid entityId, string remoteId) : CreateChange(entityId), IPolyType { public string RemoteId { get; set; } = remoteId; public override ValueTask NewEntity(Commit commit, IChangeContext context) diff --git a/src/SIL.Harmony/Resource/HarmonyResource.cs b/src/SIL.Harmony/Resource/HarmonyResource.cs index 072f29d..cfe455e 100644 --- a/src/SIL.Harmony/Resource/HarmonyResource.cs +++ b/src/SIL.Harmony/Resource/HarmonyResource.cs @@ -1,3 +1,5 @@ +using System.Diagnostics.CodeAnalysis; + namespace SIL.Harmony.Resource; public class HarmonyResource @@ -5,6 +7,8 @@ public class HarmonyResource public required Guid Id { get; init; } public string? RemoteId { get; init; } public string? LocalPath { get; init; } + [MemberNotNullWhen(true, nameof(LocalPath))] public bool Local => !string.IsNullOrEmpty(LocalPath); + [MemberNotNullWhen(true, nameof(RemoteId))] public bool Remote => !string.IsNullOrEmpty(RemoteId); -} \ No newline at end of file +} diff --git a/src/SIL.Harmony/ResourceService.cs b/src/SIL.Harmony/ResourceService.cs index c4fae85..7f5dce6 100644 --- a/src/SIL.Harmony/ResourceService.cs +++ b/src/SIL.Harmony/ResourceService.cs @@ -28,6 +28,23 @@ private void ValidateResourcesSetup() if (!_crdtConfig.Value.RemoteResourcesEnabled) throw new RemoteResourceNotEnabledException(); } + public async Task AddExistingRemoteResource(string resourcePath, + Guid clientId, + Guid resourceId, string remoteId) + { + ValidateResourcesSetup(); + var localResource = new LocalResource + { + Id = resourceId, + LocalPath = Path.GetFullPath(resourcePath) + }; + if (!localResource.FileExists()) throw new FileNotFoundException(localResource.LocalPath); + + await _dataModel.AddChange(clientId, new CreateRemoteResourceChange(localResource.Id, remoteId)); + await using var repo = await _crdtRepositoryFactory.CreateRepository(); + await repo.AddLocalResource(localResource); + } + public async Task AddLocalResource(string resourcePath, Guid clientId, Guid id = default, @@ -49,7 +66,7 @@ public async Task AddLocalResource(string resourcePath, try { - uploadResult = await resourceService.UploadResource(localResource.LocalPath); + uploadResult = await resourceService.UploadResource(localResource.Id, localResource.LocalPath); } catch (Exception e) { @@ -93,7 +110,7 @@ public async Task UploadPendingResources(Guid clientId, IRemoteResourceService r { foreach (var localResource in pendingUploads) { - var uploadResult = await remoteResourceService.UploadResource(localResource.LocalPath); + var uploadResult = await remoteResourceService.UploadResource(localResource.Id, localResource.LocalPath); changes.Add(new RemoteResourceUploadedChange(localResource.Id, uploadResult.RemoteId)); } } @@ -116,7 +133,7 @@ public async Task UploadPendingResource(Guid resourceId, Guid clientId, IRemoteR public async Task UploadPendingResource(LocalResource localResource, Guid clientId, IRemoteResourceService remoteResourceService) { ValidateResourcesSetup(); - var uploadResult = await remoteResourceService.UploadResource(localResource.LocalPath); + var uploadResult = await remoteResourceService.UploadResource(localResource.Id, localResource.LocalPath); await _dataModel.AddChange(clientId, new RemoteResourceUploadedChange(localResource.Id, uploadResult.RemoteId)); } @@ -196,4 +213,11 @@ private async Task> AllResourcesInternal() var resources = await AllResourcesInternal(); return resources.FirstOrDefault(r => r.Id == resourceId); } + + public async Task DeleteResource(Guid clientId, Guid resourceId) + { + await _dataModel.AddChange(clientId, new DeleteChange(resourceId)); + var repo = await _crdtRepositoryFactory.CreateRepository(); + await repo.DeleteLocalResource(resourceId); + } } From 7d32dcf43b1fb87b4571772049b85c19e552798d Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 14 Jul 2025 11:50:51 +0700 Subject: [PATCH 2/7] fix test mock --- src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs b/src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs index 6375c44..33a4f3b 100644 --- a/src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs +++ b/src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs @@ -26,7 +26,7 @@ public Task DownloadResource(string remoteId, string localResour private readonly Queue _throwOnUpload = new(); - public async Task UploadResource(string localPath) + public async Task UploadResource(Guid resourceId, string localPath) { await Task.Yield();//yield back to the scheduler to emulate how exceptions are thrown if (_throwOnUpload.TryPeek(out var throwOnUpload)) From 6cd0630a2be7bc4010a1ebc1c7e826054768b553 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 14 Jul 2025 13:39:34 +0700 Subject: [PATCH 3/7] fix change deserialization --- .../Resource/CreateRemoteResourcePendingUpload.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs b/src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs index 1cce929..73f2a2d 100644 --- a/src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs +++ b/src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs @@ -3,12 +3,9 @@ namespace SIL.Harmony.Resource; -public class CreateRemoteResourcePendingUploadChange: CreateChange, IPolyType +public class CreateRemoteResourcePendingUploadChange(Guid entityId) + : CreateChange(entityId), IPolyType { - public CreateRemoteResourcePendingUploadChange(Guid resourceId) : base(resourceId) - { - } - public override ValueTask NewEntity(Commit commit, IChangeContext context) { return ValueTask.FromResult(new RemoteResource From 55e3785c17ff4d04d5bb5e57ac5cedb5d1f3b73a Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Tue, 15 Jul 2025 13:08:53 +0700 Subject: [PATCH 4/7] Add tests for resource deletion functionality - Implemented tests to verify that local and remote resources are correctly deleted from the resource service. - Ensured that deleted resources are no longer retrievable from all APIs. --- .../ResourceTests/RemoteResourcesTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs b/src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs index f9d6576..59e9b84 100644 --- a/src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs +++ b/src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs @@ -207,4 +207,38 @@ public async Task CanGetAResourceGivenAnId() (await _resourceService.GetResource(localAndRemoteResource.Id)).Should().BeEquivalentTo(localAndRemoteResource); (await _resourceService.GetResource(Guid.NewGuid())).Should().BeNull(); } + + [Fact] + public async Task DeleteResource_RemovesLocalResource() + { + // Arrange: create a local resource + var (resourceId, localPath) = await SetupLocalFile("delete-local"); + (await _resourceService.GetResource(resourceId)).Should().NotBeNull(); + (await _resourceService.GetLocalResource(resourceId)).Should().NotBeNull(); + + // Act: delete the resource + await _resourceService.DeleteResource(_localClientId, resourceId); + + // Assert: resource is gone from all APIs + (await _resourceService.GetResource(resourceId)).Should().BeNull(); + (await _resourceService.GetLocalResource(resourceId)).Should().BeNull(); + (await _resourceService.AllResources()).Should().NotContain(r => r.Id == resourceId); + } + + [Fact] + public async Task DeleteResource_RemovesRemoteResource() + { + // Arrange: create a remote resource + var (resourceId, remoteId) = await SetupRemoteResource("delete-remote"); + (await _resourceService.GetResource(resourceId)).Should().NotBeNull(); + (await _resourceService.GetLocalResource(resourceId)).Should().BeNull(); + + // Act: delete the resource + await _resourceService.DeleteResource(_localClientId, resourceId); + + // Assert: resource is gone from all APIs + (await _resourceService.GetResource(resourceId)).Should().BeNull(); + (await _resourceService.GetLocalResource(resourceId)).Should().BeNull(); + (await _resourceService.AllResources()).Should().NotContain(r => r.Id == resourceId); + } } From 90e923c7ec6a561fc68d947abe84292b363aaa75 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Tue, 15 Jul 2025 14:13:51 +0700 Subject: [PATCH 5/7] Update src/SIL.Harmony/ResourceService.cs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- src/SIL.Harmony/ResourceService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SIL.Harmony/ResourceService.cs b/src/SIL.Harmony/ResourceService.cs index 7f5dce6..21baea4 100644 --- a/src/SIL.Harmony/ResourceService.cs +++ b/src/SIL.Harmony/ResourceService.cs @@ -217,7 +217,7 @@ private async Task> AllResourcesInternal() public async Task DeleteResource(Guid clientId, Guid resourceId) { await _dataModel.AddChange(clientId, new DeleteChange(resourceId)); - var repo = await _crdtRepositoryFactory.CreateRepository(); + await using var repo = await _crdtRepositoryFactory.CreateRepository(); await repo.DeleteLocalResource(resourceId); } } From 10f23e2a37f5dd8f967c0de3eb4321069a3df4b7 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Mon, 21 Jul 2025 16:11:56 +0700 Subject: [PATCH 6/7] fix issue trying to have 2 transactions on the same db at once --- src/SIL.Harmony/Db/CrdtRepository.cs | 5 +++++ src/SIL.Harmony/ResourceService.cs | 5 +---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/SIL.Harmony/Db/CrdtRepository.cs b/src/SIL.Harmony/Db/CrdtRepository.cs index 91d6ed3..a7145cb 100644 --- a/src/SIL.Harmony/Db/CrdtRepository.cs +++ b/src/SIL.Harmony/Db/CrdtRepository.cs @@ -30,6 +30,11 @@ public async Task Execute(Func> func) await using var repo = await CreateRepository(); return await func(repo); } + public async Task Execute(Func func) + { + await using var repo = await CreateRepository(); + await func(repo); + } public async ValueTask Execute(Func> func) { diff --git a/src/SIL.Harmony/ResourceService.cs b/src/SIL.Harmony/ResourceService.cs index 21baea4..6f1ff66 100644 --- a/src/SIL.Harmony/ResourceService.cs +++ b/src/SIL.Harmony/ResourceService.cs @@ -51,15 +51,12 @@ public async Task AddLocalResource(string resourcePath, IRemoteResourceService? resourceService = null) { ValidateResourcesSetup(); - await using var repo = await _crdtRepositoryFactory.CreateRepository(); var localResource = new LocalResource { Id = id == default ? Guid.NewGuid() : id, LocalPath = Path.GetFullPath(resourcePath) }; if (!localResource.FileExists()) throw new FileNotFoundException(localResource.LocalPath); - await using var transaction = await repo.BeginTransactionAsync(); - await repo.AddLocalResource(localResource); UploadResult? uploadResult = null; if (resourceService is not null) { @@ -83,7 +80,7 @@ public async Task AddLocalResource(string resourcePath, await _dataModel.AddChange(clientId, new CreateRemoteResourcePendingUploadChange(localResource.Id)); } - await transaction.CommitAsync(); + await _crdtRepositoryFactory.Execute(repo => repo.AddLocalResource(localResource)); return new HarmonyResource { Id = localResource.Id, From 4305ac0d38a74e80bea9286d3588c324310fadf9 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Tue, 22 Jul 2025 09:50:43 +0700 Subject: [PATCH 7/7] return early if there are no pending resources to upload --- src/SIL.Harmony/ResourceService.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/SIL.Harmony/ResourceService.cs b/src/SIL.Harmony/ResourceService.cs index 6f1ff66..344765c 100644 --- a/src/SIL.Harmony/ResourceService.cs +++ b/src/SIL.Harmony/ResourceService.cs @@ -102,6 +102,7 @@ public async Task UploadPendingResources(Guid clientId, IRemoteResourceService r { ValidateResourcesSetup(); var pendingUploads = await ListResourcesPendingUpload(); + if (pendingUploads is []) return; var changes = new List(pendingUploads.Length); try {