Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add remote resource support #2

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions src/SIL.Harmony.Core/EntityNotFoundException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
namespace SIL.Harmony.Core;

public class EntityNotFoundException(string message) : Exception(message);
27 changes: 27 additions & 0 deletions src/SIL.Harmony.Core/IRemoteResourceService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace SIL.Harmony.Core;

/// <summary>
/// interface to facilitate downloading of resources, typically implemented in application code
/// the remote Id is opaque to the CRDT lib and could be a URL or some other identifier provided by the backend
/// the local path returned for the application code to use as required, it could be a URL if needed also.
/// </summary>
public interface IRemoteResourceService
{
/// <summary>
/// instructs application code to download a resource from the remote server
/// the service is responsible for downloading the resource and returning the local path
/// </summary>
/// <param name="remoteId">ID used to identify the remote resource, could be a URL</param>
/// <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);
myieye marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// upload a resource to the remote server
/// </summary>
/// <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);
hahn-kev marked this conversation as resolved.
Show resolved Hide resolved
}

public record DownloadResult(string LocalPath);
public record UploadResult(string RemoteId);
15 changes: 15 additions & 0 deletions src/SIL.Harmony.Sample/Changes/AddWordImageChange.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using SIL.Harmony.Changes;
using SIL.Harmony.Entities;
using SIL.Harmony.Sample.Models;

namespace SIL.Harmony.Sample.Changes;

public class AddWordImageChange(Guid entityId, Guid imageId) : EditChange<Word>(entityId), ISelfNamedType<AddWordImageChange>
{
public Guid ImageId { get; } = imageId;

public override async ValueTask ApplyChange(Word entity, ChangeContext context)
{
if (!await context.IsObjectDeleted(ImageId)) entity.ImageResourceId = ImageId;
}
}
2 changes: 2 additions & 0 deletions src/SIL.Harmony.Sample/CrdtSampleKernel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi
services.AddCrdtData<SampleDbContext>(config =>
{
config.EnableProjectedTables = true;
config.AddRemoteResourceEntity();
config.ChangeTypeListBuilder
.Add<NewWordChange>()
.Add<NewDefinitionChange>()
Expand All @@ -44,6 +45,7 @@ public static IServiceCollection AddCrdtDataSample(this IServiceCollection servi
.Add<SetWordTextChange>()
.Add<SetWordNoteChange>()
.Add<AddAntonymReferenceChange>()
.Add<AddWordImageChange>()
.Add<SetOrderChange<Definition>>()
.Add<SetDefinitionPartOfSpeechChange>()
.Add<DeleteChange<Word>>()
Expand Down
9 changes: 8 additions & 1 deletion src/SIL.Harmony.Sample/Models/Word.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,17 @@ public class Word : IObjectBase<Word>
public Guid Id { get; init; }
public DateTimeOffset? DeletedAt { get; set; }
public Guid? AntonymId { get; set; }
public Guid? ImageResourceId { get; set; }

public Guid[] GetReferences()
{
return AntonymId is null ? [] : [AntonymId.Value];
return Refs().ToArray();

IEnumerable<Guid> Refs()
{
if (AntonymId.HasValue) yield return AntonymId.Value;
if (ImageResourceId.HasValue) yield return ImageResourceId.Value;
}
}

public void RemoveReference(Guid id, Commit commit)
Expand Down
35 changes: 35 additions & 0 deletions src/SIL.Harmony.Tests/DbContextTests.VerifyModel.verified.txt
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,40 @@
Relational:TableName: Snapshots
Relational:ViewName:
Relational:ViewSchema:
EntityType: LocalResource
Properties:
Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
LocalPath (string) Required
Keys:
Id PK
Annotations:
DiscriminatorProperty:
Relational:FunctionName:
Relational:Schema:
Relational:SqlQuery:
Relational:TableName: LocalResource
Relational:ViewName:
Relational:ViewSchema:
EntityType: RemoteResource
Properties:
Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
DeletedAt (DateTimeOffset?)
RemoteId (string)
SnapshotId (no field, Guid?) Shadow FK Index
Keys:
Id PK
Foreign keys:
RemoteResource {'SnapshotId'} -> ObjectSnapshot {'Id'} Unique SetNull
Indexes:
SnapshotId Unique
Annotations:
DiscriminatorProperty:
Relational:FunctionName:
Relational:Schema:
Relational:SqlQuery:
Relational:TableName: RemoteResource
Relational:ViewName:
Relational:ViewSchema:
EntityType: Definition
Properties:
Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
Expand Down Expand Up @@ -133,6 +167,7 @@
Id (Guid) Required PK AfterSave:Throw ValueGenerated.OnAdd
AntonymId (Guid?)
DeletedAt (DateTimeOffset?)
ImageResourceId (Guid?)
Note (string)
SnapshotId (no field, Guid?) Shadow FK Index
Text (string) Required
Expand Down
164 changes: 164 additions & 0 deletions src/SIL.Harmony.Tests/ResourceTests/RemoteResourcesTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
using System.Runtime.CompilerServices;
using Microsoft.Extensions.DependencyInjection;
using SIL.Harmony.Resource;

namespace SIL.Harmony.Tests.ResourceTests;

public class RemoteResourcesTests : DataModelTestBase
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In your tests you always know before hand if you're dealing with a local or remote resource.

I feel like maybe there should be a public Resource GetResource() method where Resource aggregates the local and remote resource looks something like:

Resource
{
Id,
LocalPath: string?,
RemoteId: string?,
IsLocal: bool,
IsRemote: bool,
}

I suppose a Harmony user could implement it themselves. A join is probably trivial if projected tables are enabled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that's a good idea

Copy link
Contributor

@myieye myieye Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a slight misunderstanding.

You added a GetResources() (plural). My suggestion was for looking up a single resource.
I'm thinking about the case where I have a specific Resource ID, but I don't know if it's local or remote (or both), so I'd like to be able to get a CrdtResource just for that one resource.
a GetResource() would also cover up the fact that local and remote resources are queried differently, which would be a small bonus.

Can we do the join in SQL? I guess it doesn't matter for GetResources() (plural), because everything ends up in memory anyway.

Also I wonder if CrdtResource is a misleading name: the local-resources are explicitly non-crdt values, because they're local only. 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok, that makes sense, yeah I can easily make that API (probably on top of the one I just made for now).

As for the name. Yeah that's true, I don't want to use just Resource as the class name as that's way too generic and there's other dotnet classes with the same name so I don't want the confusion. So maybe HarmonyResource?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, you're right Resource is heavily used 😆.
I like HarmonyResource except for the fact that it would be the first type to use the word Harmony in it. 🤔

Some more ideas:

  • UnifiedResource
  • HybridResource

Hybrid sounds coolest, but I think it almost sounds too cool and fancy.
Personally, I think I'd opt for UnifiedResource.

{
private RemoteServiceMock _remoteServiceMock = new();
private ResourceService _resourceService => _services.GetRequiredService<ResourceService>();

public RemoteResourcesTests()
{
}

private string CreateFile(string contents, [CallerMemberName] string fileName = "")
{
var filePath = Path.GetFullPath(fileName + ".txt");
File.WriteAllText(filePath, contents);
return filePath;
}

private async Task<(Guid resourceId, string remoteId)> SetupRemoteResource(string fileContents)
{
var remoteId = _remoteServiceMock.CreateRemoteResource(fileContents);
var resourceId = Guid.NewGuid();
await DataModel.AddChange(_localClientId, new CreateRemoteResourceChange(resourceId, remoteId));
return (resourceId, remoteId);
}

private async Task<(Guid resourceId, string localPath)> SetupLocalFile(string contents, [CallerMemberName] string fileName = "")
{
var file = CreateFile(contents, fileName);
//because resource service is null the file is not uploaded
var crdtResource = await _resourceService.AddLocalResource(file, _localClientId, resourceService: null);
return (crdtResource.Id, file);
}

[Fact]
public async Task CreatingAResourceResultsInPendingLocalResources()
{
var (_, file) = await SetupLocalFile("contents");

//act
var pending = await _resourceService.ListResourcesPendingUpload();


pending.Should().ContainSingle().Which.LocalPath.Should().Be(file);
}

[Fact]
public async Task ResourcesNotLocalShouldShowUpAsNotDownloaded()
{
var (resourceId, remoteId) = await SetupRemoteResource("test");

//act
var pending = await _resourceService.ListResourcesPendingDownload();


var remoteResource = pending.Should().ContainSingle().Subject;
remoteResource.RemoteId.Should().Be(remoteId);
remoteResource.Id.Should().Be(resourceId);
}

[Fact]
public async Task CanUploadFileToRemote()
{
var fileContents = "resource";
var localFile = CreateFile(fileContents);

//act
var crdtResource = await _resourceService.AddLocalResource(localFile, _localClientId, resourceService: _remoteServiceMock);


var resource = await DataModel.GetLatest<RemoteResource>(crdtResource.Id);
ArgumentNullException.ThrowIfNull(resource);
ArgumentNullException.ThrowIfNull(resource.RemoteId);
_remoteServiceMock.ReadFile(resource.RemoteId).Should().Be(fileContents);
var pendingUpload = await _resourceService.ListResourcesPendingUpload();
pendingUpload.Should().BeEmpty();
}

[Fact]
public async Task WillUploadMultiplePendingLocalFilesAtOnce()
{
await SetupLocalFile("file1", "file1");
await SetupLocalFile("file2", "file2");

//act
await _resourceService.UploadPendingResources(_localClientId, _remoteServiceMock);


_remoteServiceMock.ListRemoteFiles()
.Select(Path.GetFileName)
.Should()
.Contain(["file1.txt", "file2.txt"]);
}

[Fact]
public async Task CanDownloadFileFromRemote()
{
var fileContents = "resource";
var (resourceId, _) = await SetupRemoteResource(fileContents);

//act
var localResource = await _resourceService.DownloadResource(resourceId, _remoteServiceMock);


localResource.Id.Should().Be(resourceId);
var actualFileContents = await File.ReadAllTextAsync(localResource.LocalPath);
actualFileContents.Should().Be(fileContents);
var pendingDownloads = await _resourceService.ListResourcesPendingDownload();
pendingDownloads.Should().BeEmpty();
}

[Fact]
public async Task CanGetALocalResourceGivenAnId()
{
var file = CreateFile("resource");
//because resource service is null the file is not uploaded
var crdtResource = await _resourceService.AddLocalResource(file, _localClientId, resourceService: null);

//act
var localResource = await _resourceService.GetLocalResource(crdtResource.Id);


localResource.Should().NotBeNull();
localResource!.LocalPath.Should().Be(file);
}

[Fact]
public async Task LocalResourceIsNullIfNotDownloaded()
{
var (resourceId, _) = await SetupRemoteResource("test");
var localResource = await _resourceService.GetLocalResource(resourceId);
localResource.Should().BeNull();
}

[Fact]
public async Task CanListAllResources()
{
var (localResourceId, localResourcePath) = await SetupLocalFile("localOnly", "localOnly.txt");
var (remoteResourceId, remoteId) = await SetupRemoteResource("remoteOnly");
var localAndRemoteResource = await _resourceService.AddLocalResource(CreateFile("localAndRemove"), _localClientId, resourceService: _remoteServiceMock);

var crdtResources = await _resourceService.AllResources();
crdtResources.Should().BeEquivalentTo(
[
new CrdtResource
{
Id = localResourceId,
LocalPath = localResourcePath,
RemoteId = null
},
new CrdtResource
{
Id = remoteResourceId,
LocalPath = null,
RemoteId = remoteId
},
localAndRemoteResource
]);
}
}
45 changes: 45 additions & 0 deletions src/SIL.Harmony.Tests/ResourceTests/RemoteServiceMock.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using SIL.Harmony.Core;

namespace SIL.Harmony.Tests.ResourceTests;

public class RemoteServiceMock : IRemoteResourceService
{
public static readonly string RemotePath = Directory.CreateTempSubdirectory("RemoteServiceMock").FullName;

/// <summary>
/// directly creates a remote resource
/// </summary>
/// <returns>the remote id</returns>
public string CreateRemoteResource(string contents)
{
var filePath = Path.Combine(RemotePath, Guid.NewGuid().ToString("N") + ".txt");
File.WriteAllText(filePath, contents);
return filePath;
}

public Task<DownloadResult> DownloadResource(string remoteId, string localResourceCachePath)
{
var fileName = Path.GetFileName(remoteId);
var localPath = Path.Combine(localResourceCachePath, fileName);
Directory.CreateDirectory(localResourceCachePath);
File.Copy(remoteId, localPath);
return Task.FromResult(new DownloadResult(localPath));
}

public Task<UploadResult> UploadResource(string localPath)
{
var remoteId = Path.Combine(RemotePath, Path.GetFileName(localPath));
File.Copy(localPath, remoteId);
return Task.FromResult(new UploadResult(remoteId));
}

public string ReadFile(string remoteId)
{
return File.ReadAllText(remoteId);
}

public IEnumerable<string> ListRemoteFiles()
{
return Directory.GetFiles(RemotePath);
}
}
41 changes: 41 additions & 0 deletions src/SIL.Harmony.Tests/ResourceTests/WordResourceTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using System.Runtime.CompilerServices;
using Microsoft.Extensions.DependencyInjection;
using SIL.Harmony.Sample.Changes;
using SIL.Harmony.Sample.Models;

namespace SIL.Harmony.Tests.ResourceTests;

public class WordResourceTests: DataModelTestBase
{
private RemoteServiceMock _remoteServiceMock = new();
private ResourceService _resourceService => _services.GetRequiredService<ResourceService>();
private readonly Guid _entity1Id = Guid.NewGuid();

private string CreateFile(string contents, [CallerMemberName] string fileName = "")
{
var filePath = Path.GetFullPath(fileName + ".txt");
File.WriteAllText(filePath, contents);
return filePath;
}

[Fact]
public async Task CanReferenceAResourceFromAWord()
{
await WriteNextChange(SetWord(_entity1Id, "test-value"));
var imageFile = CreateFile("not image data");
//set commit date for add local resource
MockTimeProvider.SetNextDateTime(NextDate());
var resource = await _resourceService.AddLocalResource(imageFile, Guid.NewGuid(), resourceService: _remoteServiceMock);
await WriteNextChange(new AddWordImageChange(_entity1Id, resource.Id));

var word = await DataModel.GetLatest<Word>(_entity1Id);
word.Should().NotBeNull();
word!.ImageResourceId.Should().Be(resource.Id);


var localResource = await _resourceService.GetLocalResource(word.ImageResourceId!.Value);
localResource.Should().NotBeNull();
localResource!.LocalPath.Should().Be(imageFile);
(await File.ReadAllTextAsync(localResource.LocalPath)).Should().Be("not image data");
}
}
2 changes: 1 addition & 1 deletion src/SIL.Harmony/Changes/EditChange.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
namespace SIL.Harmony.Changes;
namespace SIL.Harmony.Changes;

public abstract class EditChange<T>(Guid entityId) : Change<T>(entityId)
where T : class
Expand Down
Loading