Resource update (#44)

* add some required apis to harmony

* 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.

* fix issue trying to have 2 transactions on the same db at once

* return early if there are no pending resources to upload

---------

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Author: hahn-kev

src/SIL.Harmony.Core/IRemoteResourceService.cs

@@ -15,12 +15,14 @@ public interface IRemoteResourceService
     /// <param name="localResourceCachePath">path defined by the CRDT config where the resource should be stored</param>
     /// <returns>download result containing the path to the downloaded file, this is stored in the local db and not synced</returns>
     Task<DownloadResult> DownloadResource(string remoteId, string localResourceCachePath);
+
     /// <summary>
     /// upload a resource to the remote server
     /// </summary>
+    /// <param name="resourceId">id of the resource in the CRDT</param>
     /// <param name="localPath">full path to the resource on the local machine</param>
     /// <returns>an upload result with the remote id, the id will be stored and transmitted to other clients so they can also download the resource</returns>
-    Task<UploadResult> UploadResource(string localPath);
+    Task<UploadResult> UploadResource(Guid resourceId, string localPath);
 }
 
 public record DownloadResult(string LocalPath);

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);
+    }
 }

src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs

@@ -26,7 +26,7 @@ public Task<DownloadResult> DownloadResource(string remoteId, string localResour
 
     private readonly Queue<string> _throwOnUpload = new();
 
-    public async Task<UploadResult> UploadResource(string localPath)
+    public async Task<UploadResult> 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))

src/SIL.Harmony/Db/CrdtRepository.cs

@@ -30,6 +30,11 @@ public async Task<T> Execute<T>(Func<CrdtRepository, Task<T>> func)
         await using var repo = await CreateRepository();
         return await func(repo);
     }
+    public async Task Execute(Func<CrdtRepository, Task> func)
+    {
+        await using var repo = await CreateRepository();
+        await func(repo);
+    }
 
     public async ValueTask<T> Execute<T>(Func<CrdtRepository, ValueTask<T>> func)
     {
@@ -390,6 +395,11 @@ public async Task AddLocalResource(LocalResource localResource)
         await _dbContext.SaveChangesAsync();
     }
 
+    public async Task DeleteLocalResource(Guid id)
+    {
+        await _dbContext.Set<LocalResource>().Where(r => r.Id == id).ExecuteDeleteAsync();
+    }
+
     public IAsyncEnumerable<LocalResource> LocalResourcesByIds(IEnumerable<Guid> resourceIds)
     {
         return _dbContext.Set<LocalResource>().Where(r => resourceIds.Contains(r.Id)).AsAsyncEnumerable();

src/SIL.Harmony/Resource/CreateRemoteResourceChange.cs

@@ -3,7 +3,7 @@
 
 namespace SIL.Harmony.Resource;
 
-public class CreateRemoteResourceChange(Guid resourceId, string remoteId) : CreateChange<RemoteResource>(resourceId), IPolyType
+public class CreateRemoteResourceChange(Guid entityId, string remoteId) : CreateChange<RemoteResource>(entityId), IPolyType
 {
     public string RemoteId { get; set; } = remoteId;
     public override ValueTask<RemoteResource> NewEntity(Commit commit, IChangeContext context)

src/SIL.Harmony/Resource/CreateRemoteResourcePendingUpload.cs

@@ -3,12 +3,9 @@
 
 namespace SIL.Harmony.Resource;
 
-public class CreateRemoteResourcePendingUploadChange: CreateChange<RemoteResource>, IPolyType
+public class CreateRemoteResourcePendingUploadChange(Guid entityId)
+    : CreateChange<RemoteResource>(entityId), IPolyType
 {
-    public CreateRemoteResourcePendingUploadChange(Guid resourceId) : base(resourceId)
-    {
-    }
-
     public override ValueTask<RemoteResource> NewEntity(Commit commit, IChangeContext context)
     {
         return ValueTask.FromResult(new RemoteResource

src/SIL.Harmony/Resource/HarmonyResource.cs

@@ -1,10 +1,14 @@
+using System.Diagnostics.CodeAnalysis;
+
 namespace SIL.Harmony.Resource;
 
 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);
-}
+}

src/SIL.Harmony/ResourceService.cs

@@ -28,28 +28,42 @@ 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<HarmonyResource> AddLocalResource(string resourcePath,
         Guid clientId,
         Guid id = default,
         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)
         {
             try
             {
 
-                uploadResult = await resourceService.UploadResource(localResource.LocalPath);
+                uploadResult = await resourceService.UploadResource(localResource.Id, localResource.LocalPath);
             }
             catch (Exception e)
             {
@@ -66,7 +80,7 @@ public async Task<HarmonyResource> 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,
@@ -88,12 +102,13 @@ public async Task UploadPendingResources(Guid clientId, IRemoteResourceService r
     {
         ValidateResourcesSetup();
         var pendingUploads = await ListResourcesPendingUpload();
+        if (pendingUploads is []) return;
         var changes = new List<IChange>(pendingUploads.Length);
         try
         {
             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 +131,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 +211,11 @@ private async Task<IEnumerable<HarmonyResource>> 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<RemoteResource>(resourceId));
+        await using var repo = await _crdtRepositoryFactory.CreateRepository();
+        await repo.DeleteLocalResource(resourceId);
+    }
 }