Resolve Damien's unresolved comments (#562)

* Resolve reviewer comments * Update from reviewer (verbal) comments.
sillsdev · Dec 5, 2024 · a4ac4d8 · a4ac4d8
1 parent 1daa05b
commit a4ac4d8
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 41 deletions.
diff --git a/src/Serval/src/Serval.Client/Client.g.cs b/src/Serval/src/Serval.Client/Client.g.cs
@@ -7073,15 +7073,28 @@ public partial class Corpus
     [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
     public partial class CorpusFile
     {
-        [Newtonsoft.Json.JsonProperty("fileId", Required = Newtonsoft.Json.Required.Always)]
-        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
-        public string FileId { get; set; } = default!;
+        [Newtonsoft.Json.JsonProperty("file", Required = Newtonsoft.Json.Required.Always)]
+        [System.ComponentModel.DataAnnotations.Required]
+        public ResourceLink File { get; set; } = new ResourceLink();
 
         [Newtonsoft.Json.JsonProperty("textId", Required = Newtonsoft.Json.Required.Default, NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)]
         public string? TextId { get; set; } = default!;
 
     }
 
+    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
+    public partial class ResourceLink
+    {
+        [Newtonsoft.Json.JsonProperty("id", Required = Newtonsoft.Json.Required.Always)]
+        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
+        public string Id { get; set; } = default!;
+
+        [Newtonsoft.Json.JsonProperty("url", Required = Newtonsoft.Json.Required.Always)]
+        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
+        public string Url { get; set; } = default!;
+
+    }
+
     [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
     public partial class CorpusConfig
     {
@@ -7408,19 +7421,6 @@ public partial class TranslationCorpus
 
     }
 
-    [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
-    public partial class ResourceLink
-    {
-        [Newtonsoft.Json.JsonProperty("id", Required = Newtonsoft.Json.Required.Always)]
-        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
-        public string Id { get; set; } = default!;
-
-        [Newtonsoft.Json.JsonProperty("url", Required = Newtonsoft.Json.Required.Always)]
-        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
-        public string Url { get; set; } = default!;
-
-    }
-
     [System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
     public partial class TranslationCorpusFile
     {

diff --git a/src/Serval/src/Serval.DataFiles/Consumers/GetCorpusConsumer.cs b/src/Serval/src/Serval.DataFiles/Consumers/GetCorpusConsumer.cs
@@ -14,19 +14,24 @@ public async Task Consume(ConsumeContext<GetCorpus> context)
                 context.Message.Owner,
                 context.CancellationToken
             );
+            IEnumerable<string> corpusFileIds = corpus.Files.Select(f => f.FileRef);
+            IDictionary<string, DataFile> corpusDataFilesDict = (
+                await _dataFileService.GetAllAsync(corpusFileIds, context.CancellationToken)
+            ).ToDictionary(f => f.Id);
+
             await context.RespondAsync(
                 new CorpusResult
                 {
                     CorpusId = corpus.Id,
                     Name = corpus.Name,
                     Language = corpus.Language,
-                    Files = await Task.WhenAll(
-                        corpus.Files.Select(async f => new CorpusFileResult
+                    Files = corpus
+                        .Files.Select(f => new CorpusFileResult
                         {
-                            TextId = f.TextId!,
-                            File = Map(await _dataFileService.GetAsync(f.FileId))
+                            TextId = f.TextId ?? corpusDataFilesDict[f.FileRef].Name,
+                            File = Map(corpusDataFilesDict[f.FileRef])
                         })
-                    )
+                        .ToList()
                 }
             );
         }

diff --git a/src/Serval/src/Serval.DataFiles/Contracts/CorpusFileDto.cs b/src/Serval/src/Serval.DataFiles/Contracts/CorpusFileDto.cs
@@ -2,6 +2,6 @@ namespace Serval.DataFiles.Contracts;
 
 public record CorpusFileDto
 {
-    public required string FileId { get; init; }
+    public required ResourceLinkDto File { get; init; }
     public string? TextId { get; init; }
 }
diff --git a/src/Serval/src/Serval.DataFiles/Controllers/CorporaController.cs b/src/Serval/src/Serval.DataFiles/Controllers/CorporaController.cs
@@ -177,7 +177,7 @@ CancellationToken cancellationToken
             DataFile? dataFile = await _dataFileService.GetAsync(file.FileId, cancellationToken);
             if (dataFile == null)
                 throw new InvalidOperationException($"DataFile with id {file.FileId} does not exist.");
-            dataFiles.Add(new CorpusFile { FileId = file.FileId, TextId = file.TextId });
+            dataFiles.Add(new CorpusFile { FileRef = file.FileId, TextId = file.TextId });
         }
         return dataFiles;
     }
@@ -197,6 +197,14 @@ private CorpusDto Map(Corpus source)
 
     private CorpusFileDto Map(CorpusFile source)
     {
-        return new CorpusFileDto { FileId = source.FileId, TextId = source.TextId };
+        return new CorpusFileDto
+        {
+            File = new ResourceLinkDto
+            {
+                Id = source.FileRef,
+                Url = _urlService.GetUrl(Endpoints.GetDataFile, new { id = source.FileRef })
+            },
+            TextId = source.TextId
+        };
     }
 }
diff --git a/src/Serval/src/Serval.DataFiles/Models/CorpusFile.cs b/src/Serval/src/Serval.DataFiles/Models/CorpusFile.cs
@@ -2,6 +2,6 @@ namespace Serval.DataFiles.Models;
 
 public record CorpusFile
 {
-    public required string FileId { get; init; }
+    public required string FileRef { get; init; }
     public string? TextId { get; init; }
 }
diff --git a/src/Serval/src/Serval.DataFiles/Services/CorpusService.cs b/src/Serval/src/Serval.DataFiles/Services/CorpusService.cs
@@ -2,14 +2,13 @@ namespace Serval.DataFiles.Services;
 
 public class CorpusService(
     IRepository<Corpus> corpora,
+    IRepository<DataFile> dataFiles,
     IDataAccessContext dataAccessContext,
-    IDataFileService dataFileService,
     IScopedMediator mediator
 ) : OwnedEntityServiceBase<Corpus>(corpora), ICorpusService
 {
     private readonly IDataAccessContext _dataAccessContext = dataAccessContext;
-    private readonly IDataFileService _dataFileService = dataFileService;
-
+    private readonly IRepository<DataFile> _dataFiles = dataFiles;
     private readonly IScopedMediator _mediator = mediator;
 
     public async Task<Corpus> GetAsync(string id, string owner, CancellationToken cancellationToken = default)
@@ -36,17 +35,21 @@ public async Task<Corpus> UpdateAsync(
                 );
                 if (corpus is null)
                     throw new EntityNotFoundException($"Could not find Corpus '{id}.");
+                HashSet<string> corpusFileIds = corpus.Files.Select(f => f.FileRef).ToHashSet();
+                IDictionary<string, DataFile> corpusDataFilesDict = (
+                    await _dataFiles.GetAllAsync(f => corpusFileIds.Contains(f.Id), ct)
+                ).ToDictionary(f => f.Id);
                 await _mediator.Publish(
                     new CorpusUpdated
                     {
                         CorpusId = corpus.Id,
-                        Files = await Task.WhenAll(
-                            corpus.Files.Select(async f => new CorpusFileResult
+                        Files = corpus
+                            .Files.Select(f => new CorpusFileResult
                             {
-                                TextId = f.TextId!,
-                                File = Map(await _dataFileService.GetAsync(f.FileId))
+                                TextId = f.TextId ?? corpusDataFilesDict[f.FileRef].Name,
+                                File = Map(corpusDataFilesDict[f.FileRef])
                             })
-                        )
+                            .ToList()
                     },
                     ct
                 );

diff --git a/src/Serval/src/Serval.DataFiles/Services/IDataFileService.cs b/src/Serval/src/Serval.DataFiles/Services/IDataFileService.cs
@@ -3,6 +3,7 @@
 public interface IDataFileService
 {
     Task<IEnumerable<DataFile>> GetAllAsync(string owner, CancellationToken cancellationToken = default);
+    Task<IEnumerable<DataFile>> GetAllAsync(IEnumerable<string> ids, CancellationToken cancellationToken = default);
     Task<DataFile> GetAsync(string id, CancellationToken cancellationToken = default);
     Task<DataFile> GetAsync(string id, string owner, CancellationToken cancellationToken = default);
     Task CreateAsync(DataFile dataFile, Stream stream, CancellationToken cancellationToken = default);

diff --git a/src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs b/src/Serval/src/Serval.Shared/Services/EntityServiceBase.cs
@@ -13,6 +13,15 @@ public virtual async Task<T> GetAsync(string id, CancellationToken cancellationT
         return entity;
     }
 
+    public virtual async Task<IEnumerable<T>> GetAllAsync(
+        IEnumerable<string> ids,
+        CancellationToken cancellationToken = default
+    )
+    {
+        HashSet<string> idSet = ids.ToHashSet();
+        return await Entities.GetAllAsync(e => idSet.Contains(e.Id), cancellationToken);
+    }
+
     public virtual async Task<T> CreateAsync(T entity, CancellationToken cancellationToken = default)
     {
         await Entities.InsertAsync(entity, cancellationToken);

diff --git a/src/Serval/src/Serval.Translation/Consumers/CorpusUpdatedConsumer.cs b/src/Serval/src/Serval.Translation/Consumers/CorpusUpdatedConsumer.cs
@@ -18,7 +18,7 @@ private static CorpusFile Map(CorpusFileResult corpusFile)
         return new CorpusFile
         {
             Id = corpusFile.File.DataFileId,
-            TextId = corpusFile.TextId,
+            TextId = corpusFile.TextId ?? corpusFile.File.Name,
             Filename = corpusFile.File.Filename,
             Format = corpusFile.File.Format,
         };

diff --git a/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs b/src/Serval/src/Serval.Translation/Controllers/TranslationEnginesController.cs
@@ -1294,7 +1294,7 @@ CancellationToken cancellationToken
                                 Id = f.File.DataFileId,
                                 Filename = f.File.Filename,
                                 Format = f.File.Format,
-                                TextId = f.TextId
+                                TextId = f.TextId ?? f.File.Name
                             })
                             .ToList(),
                     }

diff --git a/src/Serval/test/Serval.ApiServer.IntegrationTests/TranslationEngineTests.cs b/src/Serval/test/Serval.ApiServer.IntegrationTests/TranslationEngineTests.cs
@@ -184,21 +184,21 @@ public async Task SetUp()
             Id = SOURCE_CORPUS_ID_1,
             Language = "en",
             Owner = "client1",
-            Files = [new() { FileId = srcFile.Id, TextId = "all" }]
+            Files = [new() { FileRef = srcFile.Id, TextId = "all" }]
         };
         var srcCorpus2 = new DataFiles.Models.Corpus
         {
             Id = SOURCE_CORPUS_ID_2,
             Language = "en",
             Owner = "client1",
-            Files = [new() { FileId = srcFile.Id, TextId = "all" }]
+            Files = [new() { FileRef = srcFile.Id, TextId = "all" }]
         };
         var trgCorpus = new DataFiles.Models.Corpus
         {
             Id = TARGET_CORPUS_ID,
             Language = "en",
             Owner = "client1",
-            Files = [new() { FileId = trgFile.Id, TextId = "all" }]
+            Files = [new() { FileRef = trgFile.Id, TextId = "all" }]
         };
         await _env.Corpora.InsertAllAsync([srcCorpus, srcCorpus2, trgCorpus]);
     }
@@ -2045,7 +2045,7 @@ public async Task DataFileUpdate_Propagated()
         DataFiles.Models.DataFile orgFileFromRepo = (await _env.DataFiles.GetAsync(FILE1_SRC_ID))!;
         DataFiles.Models.Corpus orgCorpusFromRepo = (await _env.Corpora.GetAsync(TARGET_CORPUS_ID))!;
         Assert.That(orgFileFromClient.Name, Is.EqualTo(orgFileFromRepo.Name));
-        Assert.That(orgCorpusFromRepo.Files[0].FileId, Is.EqualTo(FILE2_TRG_ID));
+        Assert.That(orgCorpusFromRepo.Files[0].FileRef, Is.EqualTo(FILE2_TRG_ID));
 
         // Update the file
         await dataFilesClient.UpdateAsync(FILE1_SRC_ID, new FileParameter(new MemoryStream([1, 2, 3]), "test.txt"));

diff --git a/src/Serval/test/Serval.DataFiles.Tests/Services/CorpusServiceTests.cs b/src/Serval/test/Serval.DataFiles.Tests/Services/CorpusServiceTests.cs
@@ -21,7 +21,7 @@ public class CorpusServiceTests
             Owner = "owner1",
             Name = "corpus1",
             Language = "en",
-            Files = new List<CorpusFile>() { new() { FileId = DefaultDataFile.Id } }
+            Files = new List<CorpusFile>() { new() { FileRef = DefaultDataFile.Id } }
         };
 
     [Test]
@@ -56,8 +56,8 @@ public TestEnvironment()
                 });
             Service = new CorpusService(
                 Corpora,
+                Substitute.For<IRepository<DataFile>>(),
                 DataAccessContext,
-                Substitute.For<IDataFileService>(),
                 Substitute.For<IScopedMediator>()
             );
         }