Skip to content
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/FwLite/FwLiteShared/Services/FwLiteProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ IServiceProvider services
DotnetService.ProjectServicesProvider => typeof(ProjectServicesProvider),
DotnetService.HistoryService => typeof(HistoryServiceJsInvokable),
DotnetService.SyncService => typeof(SyncServiceJsInvokable),
DotnetService.MediaFilesService => typeof(MediaFilesServiceJsInvokable),
DotnetService.AppLauncher => typeof(IAppLauncher),
DotnetService.TroubleshootingService => typeof(ITroubleshootingService),
DotnetService.TestingService => typeof(TestingService),
Expand Down Expand Up @@ -103,6 +104,7 @@ public enum DotnetService
ProjectServicesProvider,
HistoryService,
SyncService,
MediaFilesService,
AppLauncher,
TroubleshootingService,
TestingService,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using Microsoft.JSInterop;
using LcmCrdt.MediaServer;
using SIL.Harmony.Resource;

namespace FwLiteShared.Services;

public class MediaFilesServiceJsInvokable(LcmMediaService mediaService)
{
[JSInvokable]
public async Task<RemoteResource[]> ResourcesPendingDownload()
{
return await mediaService.ResourcesPendingDownload();
}

[JSInvokable]
public async Task<LocalResource[]> ResourcesPendingUpload()
{
return await mediaService.ResourcesPendingUpload();
}

[JSInvokable]
public async Task DownloadAllResources()
{
await mediaService.DownloadAllResources();
}

[JSInvokable]
public async Task UploadAllResources()
{
await mediaService.UploadAllResources();
}

[JSInvokable]
public async Task DownloadResources(IEnumerable<RemoteResource> resources)
{
await mediaService.DownloadResources(resources);
}

[JSInvokable]
public async Task UploadResources(IEnumerable<LocalResource> resources)
{
await mediaService.UploadResources(resources);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Expose metadata APIs through JS interop to meet PR objectives.

Per PR objectives, the backend should provide an API to fetch file metadata. LcmMediaService has GetFileMetadata(Guid fileId) and AllResources() but the invokable does not expose them, so the frontend can’t call them.

Add wrappers:

 public class MediaFilesServiceJsInvokable(LcmMediaService mediaService)
 {
+    [JSInvokable]
+    public async Task<HarmonyResource[]> AllResources()
+    {
+        return await mediaService.AllResources();
+    }
+
     [JSInvokable]
     public async Task<RemoteResource[]> ResourcesPendingDownload()
     {
         return await mediaService.ResourcesPendingDownload();
     }
@@
     public async Task UploadResources(IEnumerable<LocalResource> resources)
     {
         await mediaService.UploadResources(resources);
     }
+
+    [JSInvokable]
+    public async Task<LcmFileMetadata> GetFileMetadata(Guid fileId)
+    {
+        return await mediaService.GetFileMetadata(fileId);
+    }
 }

You’ll also need to export HarmonyResource and LcmFileMetadata via TypeGen so the TS interface gains these signatures, and update the generated IMediaFilesServiceJsInvokable plus the frontend wrapper accordingly. I can draft the TypeGen config changes if helpful.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public class MediaFilesServiceJsInvokable(LcmMediaService mediaService)
{
[JSInvokable]
public async Task<RemoteResource[]> ResourcesPendingDownload()
{
return await mediaService.ResourcesPendingDownload();
}
[JSInvokable]
public async Task<LocalResource[]> ResourcesPendingUpload()
{
return await mediaService.ResourcesPendingUpload();
}
[JSInvokable]
public async Task DownloadAllResources()
{
await mediaService.DownloadAllResources();
}
[JSInvokable]
public async Task UploadAllResources()
{
await mediaService.UploadAllResources();
}
[JSInvokable]
public async Task DownloadResources(IEnumerable<RemoteResource> resources)
{
await mediaService.DownloadResources(resources);
}
[JSInvokable]
public async Task UploadResources(IEnumerable<LocalResource> resources)
{
await mediaService.UploadResources(resources);
}
}
public class MediaFilesServiceJsInvokable(LcmMediaService mediaService)
{
[JSInvokable]
public async Task<HarmonyResource[]> AllResources()
{
return await mediaService.AllResources();
}
[JSInvokable]
public async Task<RemoteResource[]> ResourcesPendingDownload()
{
return await mediaService.ResourcesPendingDownload();
}
[JSInvokable]
public async Task<LocalResource[]> ResourcesPendingUpload()
{
return await mediaService.ResourcesPendingUpload();
}
[JSInvokable]
public async Task DownloadAllResources()
{
await mediaService.DownloadAllResources();
}
[JSInvokable]
public async Task UploadAllResources()
{
await mediaService.UploadAllResources();
}
[JSInvokable]
public async Task DownloadResources(IEnumerable<RemoteResource> resources)
{
await mediaService.DownloadResources(resources);
}
[JSInvokable]
public async Task UploadResources(IEnumerable<LocalResource> resources)
{
await mediaService.UploadResources(resources);
}
[JSInvokable]
public async Task<LcmFileMetadata> GetFileMetadata(Guid fileId)
{
return await mediaService.GetFileMetadata(fileId);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
IServiceProvider serviceProvider,
LexboxProjectService lexboxProjectService,
IEnumerable<IProjectProvider> projectProviders,
ILogger<ProjectServicesProvider> logger

Check warning on line 21 in backend/FwLite/FwLiteShared/Services/ProjectServicesProvider.cs

View workflow job for this annotation

GitHub Actions / Build FW Lite and run tests

Parameter 'logger' is unread.
): IAsyncDisposable
{
private IProjectProvider? FwDataProjectProvider =>
Expand Down Expand Up @@ -69,7 +69,8 @@
scope.Server = server;
scope.SetCrdtServices(
ActivatorUtilities.CreateInstance<HistoryServiceJsInvokable>(scopedServices),
ActivatorUtilities.CreateInstance<SyncServiceJsInvokable>(scopedServices)
ActivatorUtilities.CreateInstance<SyncServiceJsInvokable>(scopedServices),
ActivatorUtilities.CreateInstance<MediaFilesServiceJsInvokable>(scopedServices)
);
_projectScopes.TryAdd(scope, scope);
return scope;
Expand Down Expand Up @@ -173,10 +174,12 @@

public void SetCrdtServices(
HistoryServiceJsInvokable historyService,
SyncServiceJsInvokable syncService)
SyncServiceJsInvokable syncService,
MediaFilesServiceJsInvokable mediaFilesService)
{
HistoryService = DotNetObjectReference.Create(historyService);
SyncService = DotNetObjectReference.Create(syncService);
MediaFilesService = DotNetObjectReference.Create(mediaFilesService);
}

public ValueTask CleanupAsync()
Expand All @@ -191,4 +194,5 @@
public DotNetObjectReference<MiniLcmJsInvokable> MiniLcm { get; set; }
public DotNetObjectReference<HistoryServiceJsInvokable>? HistoryService { get; set; }
public DotNetObjectReference<SyncServiceJsInvokable>? SyncService { get; set; }
public DotNetObjectReference<MediaFilesServiceJsInvokable>? MediaFilesService { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using SIL.Harmony;
using SIL.Harmony.Core;
using SIL.Harmony.Db;
using SIL.Harmony.Resource;
using System.Runtime.CompilerServices;
using FwLiteShared.AppUpdate;
using FwLiteShared.Sync;
Expand Down Expand Up @@ -84,7 +85,9 @@ private static void ConfigureMiniLcmTypes(ConfigurationBuilder builder)
typeof(RichTextObjectData),

typeof(MediaFile),
typeof(LcmFileMetadata)
typeof(LcmFileMetadata),
typeof(RemoteResource),
typeof(LocalResource)
],
exportBuilder => exportBuilder.WithPublicNonStaticProperties(exportBuilder =>
{
Expand Down Expand Up @@ -112,6 +115,10 @@ private static void ConfigureMiniLcmTypes(ConfigurationBuilder builder)
.FlattenHierarchy()
.WithPublicProperties()
.WithPublicMethods(b => b.AlwaysReturnPromise().OnlyJsInvokable());
builder.ExportAsInterface<MediaFilesServiceJsInvokable>()
// .WithPublicMethods(b => b.AlwaysReturnPromise().OnlyJsInvokable());
.WithPublicMethods();
Copy link
Collaborator

Choose a reason for hiding this comment

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

ues we should always use OnlyJsInvokable for services which are executed over js like this, and in this case AlwaysReturnPromise is a good idea too.

// TODO: Does MediaFilesServiceJsInvokable need the AlwaysReturnPromise().OnlyJsInvokable() setup that MiniLcmJsInvokable needs?
builder.ExportAsEnum<SortField>().UseString();
builder.ExportAsInterfaces([
typeof(QueryOptions),
Expand Down
3 changes: 3 additions & 0 deletions backend/FwLite/LcmCrdt/MediaServer/IMediaServerClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ public interface IMediaServerClient
[Get("/api/media/{fileId}")]
Task<HttpResponseMessage> DownloadFile(Guid fileId);

[Get("/api/media/metadata/{fileId}")]
Task<HttpResponseMessage> GetFileMetadata(Guid fileId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It probably makes more sense to just use the return type of LcmFileMetadata here, and the request library will handle errors for us. DownloadFile was a special case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, thanks. That will make it easier, I won't have to jump through quite as many hoops. (It was just a couple of hoops, but still).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't gotten around to this one yet, BTW. I currently don't expose a GetFileMetadata call in the JsInvokable API (just downloading and uploading all files, or a selected list of files), so perhaps I should delete this? Although come to think of it, if we want to let users pick and choose files to download selectively then they're going to need to know at least the filename of the file, so maybe this one is needed after all.


[Post("/api/media")]
[Multipart]
Task<MediaUploadFileResponse> UploadFile(MultipartItem file,
Expand Down
98 changes: 95 additions & 3 deletions backend/FwLite/LcmCrdt/MediaServer/LcmMediaService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using LcmCrdt.RemoteSync;
using Microsoft.Extensions.Logging;
using MiniLcm.Media;
using System.Net.Http.Json;

namespace LcmCrdt.MediaServer;

Expand All @@ -24,6 +25,16 @@ public async Task<HarmonyResource[]> AllResources()
return await resourceService.AllResources();
}

public async Task<RemoteResource[]> ResourcesPendingDownload()
{
return await resourceService.ListResourcesPendingDownload();
}

public async Task<LocalResource[]> ResourcesPendingUpload()
{
return await resourceService.ListResourcesPendingUpload();
}

/// <summary>
/// should only be used in fw-headless for files which already exist in the lexbox db
/// </summary>
Expand All @@ -42,6 +53,56 @@ public async Task DeleteResource(Guid fileId)
await resourceService.DeleteResource(currentProjectService.ProjectData.ClientId, fileId);
}

public async Task<LocalResource?> DownloadResourceIfNeeded(Guid fileId)
{
var localResource = await resourceService.GetLocalResource(fileId);
if (localResource is null)
{
var connectionStatus = await httpClientProvider.ConnectionStatus();
if (connectionStatus == ConnectionStatus.Online)
{
return await resourceService.DownloadResource(fileId, this);
}
}
return localResource;
}

public async Task DownloadAllResources()
{
var connectionStatus = await httpClientProvider.ConnectionStatus();
if (connectionStatus == ConnectionStatus.Online)
{
var resources = await ResourcesPendingDownload();
foreach (var resource in resources)
{
if (resource.RemoteId is null) continue;
await resourceService.DownloadResource(resource.Id, this);
}
}
// TODO: Gracefully handle other connection statuses, e.g. "not logged in"
}

public async Task UploadAllResources()
{
await UploadResources(await ResourcesPendingUpload());
}

public async Task DownloadResources(IEnumerable<RemoteResource> resources)
{
foreach (var resource in resources)
{
await DownloadResourceIfNeeded(resource.Id);
}
}

public async Task UploadResources(IEnumerable<LocalResource> resources)
{
foreach (var resource in resources)
{
await ((IRemoteResourceService)this).UploadResource(resource.Id, resource.LocalPath);
}
}

/// <summary>
/// return a stream for the file, if it's not cached locally, it will be downloaded
/// </summary>
Expand All @@ -50,25 +111,56 @@ public async Task DeleteResource(Guid fileId)
/// <exception cref="FileNotFoundException"></exception>
public async Task<ReadFileResponse> GetFileStream(Guid fileId)
{
var localResource = await resourceService.GetLocalResource(fileId);
var localResource = await DownloadResourceIfNeeded(fileId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't like that we're not calling DownloadResourceIfNeeded 3 different times, if we want to implement retrying then that should go somewhere else, not in this method as it makes it much more messy and hard to follow.

if (localResource is null)
{
var connectionStatus = await httpClientProvider.ConnectionStatus();
if (connectionStatus == ConnectionStatus.Online)
{
localResource = await resourceService.DownloadResource(fileId, this);
// Try again, maybe earlier failure was a blip
localResource = await DownloadResourceIfNeeded(fileId);
}
else
{
return new ReadFileResponse(ReadFileResult.Offline);
}
}
//todo, consider trying to download the file again, maybe the cache was cleared
if (localResource is null || !File.Exists(localResource.LocalPath))
{
// One more attempt to download again, maybe the cache was cleared
localResource = await DownloadResourceIfNeeded(fileId);
// If still null then connection is offline or unreliable enough to consider as offline
if (localResource is null) return new ReadFileResponse(ReadFileResult.Offline);
}
// If still can't find local path then this is where we give up
if (!File.Exists(localResource.LocalPath))
throw new FileNotFoundException("Unable to find the file with Id" + fileId, localResource.LocalPath);
return new(File.OpenRead(localResource.LocalPath), Path.GetFileName(localResource.LocalPath));
}

public async Task<LcmFileMetadata> GetFileMetadata(Guid fileId)
{
var mediaClient = await MediaServerClient();
var response = await mediaClient.GetFileMetadata(fileId);
if (!response.IsSuccessStatusCode)
{
throw new Exception($"Failed to retrieve metadata for file {fileId}: {response.StatusCode} {response.ReasonPhrase}");
}
var metadata = await response.Content.ReadFromJsonAsync<LcmFileMetadata>();
if (metadata is null)
{
// Try to get content into error message, but if buffering not enabled for this request, give up
var content = "";
try
{
content = await response.Content.ReadAsStringAsync();
}
catch { } // Oh well, we tried
throw new Exception($"Failed to retrieve metadata for file {fileId}: response was in incorrect format. {content}");
}
return metadata;
}

private async Task<(Stream? stream, string? filename)> RequestMediaFile(Guid fileId)
{
var mediaClient = await MediaServerClient();
Expand Down
6 changes: 6 additions & 0 deletions frontend/viewer/src/DotnetProjectView.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
} from '$lib/dotnet-types/generated-types/FwLiteShared/Services/ISyncServiceJsInvokable';
import ProjectLoader from './ProjectLoader.svelte';
import {initProjectContext} from '$lib/project-context.svelte';
import type {IMediaFilesServiceJsInvokable} from '$lib/dotnet-types/generated-types/FwLiteShared/Services/IMediaFilesServiceJsInvokable';

const projectServicesProvider = useProjectServicesProvider();
const projectContext = initProjectContext();
Expand Down Expand Up @@ -52,11 +53,16 @@
if (projectScope.syncService) {
syncService = wrapInProxy(projectScope.syncService, DotnetService.SyncService);
}
let mediaFilesService: IMediaFilesServiceJsInvokable | undefined = undefined;
if (projectScope.mediaFilesService) {
mediaFilesService = wrapInProxy(projectScope.mediaFilesService, DotnetService.MediaFilesService);
}
const api = wrapInProxy(projectScope.miniLcm, DotnetService.MiniLcmApi);
projectContext.setup({
api,
historyService,
syncService,
mediaFilesService,
projectName,
projectCode: code,
projectType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export enum DotnetService {
ProjectServicesProvider = "ProjectServicesProvider",
HistoryService = "HistoryService",
SyncService = "SyncService",
MediaFilesService = "MediaFilesService",
AppLauncher = "AppLauncher",
TroubleshootingService = "TroubleshootingService",
TestingService = "TestingService",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* eslint-disable */
// This code was generated by a Reinforced.Typings tool.
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.

import type {IRemoteResource} from '../../SIL/Harmony/Resource/IRemoteResource';
import type {ILocalResource} from '../../SIL/Harmony/Resource/ILocalResource';

export interface IMediaFilesServiceJsInvokable
{
resourcesPendingDownload() : Promise<IRemoteResource[]>;
resourcesPendingUpload() : Promise<ILocalResource[]>;
downloadAllResources() : Promise<void>;
uploadAllResources() : Promise<void>;
downloadResources(resources: IRemoteResource[]) : Promise<void>;
uploadResources(resources: ILocalResource[]) : Promise<void>;
}
/* eslint-enable */
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ export interface IProjectScope
miniLcm: DotNet.DotNetObject;
historyService?: DotNet.DotNetObject;
syncService?: DotNet.DotNetObject;
mediaFilesService?: DotNet.DotNetObject;
}
/* eslint-enable */
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-disable */
// This code was generated by a Reinforced.Typings tool.
// Changes to this file may cause incorrect behavior and will be lost if
// the code is regenerated.

export interface ILocalResource
{
id: string;
localPath: string;
}
/* eslint-enable */
9 changes: 9 additions & 0 deletions frontend/viewer/src/lib/project-context.svelte.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import type {
import type {
ISyncServiceJsInvokable
} from '$lib/dotnet-types/generated-types/FwLiteShared/Services/ISyncServiceJsInvokable';
import type {
IMediaFilesServiceJsInvokable
} from '$lib/dotnet-types/generated-types/FwLiteShared/Services/IMediaFilesServiceJsInvokable';
import {resource, type ResourceOptions, type ResourceReturn} from 'runed';
import {SvelteMap} from 'svelte/reactivity';
import type {IProjectData} from '$lib/dotnet-types/generated-types/LcmCrdt/IProjectData';
Expand All @@ -18,6 +21,7 @@ interface ProjectContextSetup {
api: IMiniLcmJsInvokable;
historyService?: IHistoryServiceJsInvokable;
syncService?: ISyncServiceJsInvokable;
mediaFilesService?: IMediaFilesServiceJsInvokable;
projectName: string;
projectCode: string;
projectType?: 'crdt' | 'fwdata';
Expand All @@ -43,6 +47,7 @@ export class ProjectContext {
#projectData = $state<IProjectData>();
#historyService: IHistoryServiceJsInvokable | undefined = $state(undefined);
#syncService: ISyncServiceJsInvokable | undefined = $state(undefined);
#mediaFilesService: IMediaFilesServiceJsInvokable | undefined = $state(undefined);
#paratext = $state(false);
#features = resource(() => this.#api, (api) => {
if (!api) return Promise.resolve({} satisfies IMiniLcmFeatures);
Expand Down Expand Up @@ -81,6 +86,9 @@ export class ProjectContext {
public get syncService(): ISyncServiceJsInvokable | undefined {
return this.#syncService;
}
public get mediaFilesService(): IMediaFilesServiceJsInvokable | undefined {
return this.#mediaFilesService;
}
public get inParatext(): boolean {
return this.#paratext;
}
Expand All @@ -93,6 +101,7 @@ export class ProjectContext {
this.#api = args.api;
this.#historyService = args.historyService;
this.#syncService = args.syncService;
this.#mediaFilesService = args.mediaFilesService;
this.#projectName = args.projectName;
this.#projectCode = args.projectCode;
this.#projectType = args.projectType;
Expand Down
Loading
Loading