Enforce unique speaker names; Use name on export (#3018)

Backend.Tests/Controllers/LiftControllerTests.cs

@@ -454,6 +454,63 @@ public async Task TestDeletedWordsExportToLift()
             Assert.That(notFoundResult, Is.TypeOf<NotFoundObjectResult>());
         }
 
+        [Test]
+        public async Task TestExportConsentFileWithSpeakerName()
+        {
+            // Add word so there's something to export
+            await _wordRepo.Create(Util.RandomWord(_projId));
+
+            // Add speakers to project with names that will collide when sanitized
+            await _speakerRepo.Create(new Speaker { Name = "No consent!", ProjectId = _projId });
+
+            var nameNoExt = "Underscored_name";
+            var speakerNoExt = await _speakerRepo.Create(
+                new Speaker { Name = nameNoExt, ProjectId = _projId, Consent = ConsentType.Image });
+            var speakerNoExt1 = await _speakerRepo.Create(
+                new Speaker { Name = "Underscored name", ProjectId = _projId, Consent = ConsentType.Image });
+            var speakerNoExt2 = await _speakerRepo.Create(
+                new Speaker { Name = "Underscored_náme", ProjectId = _projId, Consent = ConsentType.Image });
+
+            var nameExts = "Imagination";
+            var speakerExts = await _speakerRepo.Create(
+                new Speaker { Name = nameExts, ProjectId = _projId, Consent = ConsentType.Image });
+            var speakerExts1 = await _speakerRepo.Create(
+                new Speaker { Name = "Imágination", ProjectId = _projId, Consent = ConsentType.Image });
+
+            // Create mock consent files tied to speaker ids
+            var pathNoExt = FileStorage.GenerateConsentFilePath(speakerNoExt.Id);
+            var pathNoExt1 = FileStorage.GenerateConsentFilePath(speakerNoExt1.Id);
+            var pathNoExt2 = FileStorage.GenerateConsentFilePath(speakerNoExt2.Id);
+            var expectedFileNames = new List<string> { nameNoExt, $"{nameNoExt}1", $"{nameNoExt}2" };
+
+            var ext = ".png";
+            var pathExt = FileStorage.GenerateConsentFilePath(speakerExts.Id, ext);
+            var pathExt1 = FileStorage.GenerateConsentFilePath(speakerExts1.Id, ext);
+            expectedFileNames.AddRange(new List<string> { $"{nameExts}{ext}", $"{nameExts}1{ext}" });
+
+            var mockFiles = new List<string> { pathNoExt, pathNoExt1, pathNoExt2, pathExt, pathExt1 };
+            mockFiles.ForEach(path => File.Create(path).Dispose());
+
+            // Export the project
+            var exportedFilePath = await _liftController.CreateLiftExport(_projId);
+            var exportedDirectory = FileOperations.ExtractZipFile(exportedFilePath, null);
+            var exportedProjDir = Directory.GetDirectories(exportedDirectory).First();
+
+            // Verify all consent files were copied over with speaker names
+            var consentFiles = Directory.GetFiles(Path.Combine(exportedProjDir, "consent"));
+            var consentFileNames = consentFiles.Select(path => Path.GetFileName(path)).ToList();
+            Assert.That(consentFileNames, Has.Count.EqualTo(expectedFileNames.Count));
+            foreach (var file in expectedFileNames)
+            {
+                Assert.That(consentFileNames.Contains(file));
+            };
+
+            // Delete everything
+            mockFiles.ForEach(path => File.Delete(path));
+            File.Delete(exportedFilePath);
+            Directory.Delete(exportedDirectory, true);
+        }
+
         private static RoundTripObj[] _roundTripCases =
         {
             new("Gusillaay.zip", "gsl-Qaaa-x-orth", new List<string>(), 8045),

Backend.Tests/Controllers/SpeakerControllerTests.cs

@@ -127,6 +127,22 @@ public void TestCreateSpeakerUnauthorized()
             Assert.That(result, Is.InstanceOf<ForbidResult>());
         }
 
+        [Test]
+        public void TestCreateEmptyName()
+        {
+            var result = _speakerController.CreateSpeaker(ProjId, " \n\t").Result;
+            Assert.That(result, Is.InstanceOf<BadRequestObjectResult>());
+        }
+
+        [Test]
+        public void TestCreateNameTaken()
+        {
+            var oldCount = _speakerRepo.GetAllSpeakers(ProjId).Result.Count;
+            var result = _speakerController.CreateSpeaker(ProjId, $"\n{Name} ").Result;
+            Assert.That(result, Is.InstanceOf<BadRequestObjectResult>());
+            Assert.That(_speakerRepo.GetAllSpeakers(ProjId).Result, Has.Count.EqualTo(oldCount));
+        }
+
         [Test]
         public void TestCreateSpeaker()
         {
@@ -212,10 +228,21 @@ public void TestUpdateSpeakerNameNoSpeaker()
         }
 
         [Test]
-        public void TestUpdateSpeakerNameSameName()
+        public void TestUpdateSpeakerNameEmptyName()
         {
-            var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, Name).Result;
-            Assert.That(((ObjectResult)result).StatusCode, Is.EqualTo(StatusCodes.Status304NotModified));
+            var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, " \n\t").Result;
+            Assert.That(result, Is.InstanceOf<BadRequestObjectResult>());
+        }
+
+        [Test]
+        public void TestUpdateSpeakerNameNameTaken()
+        {
+            var result = _speakerController.UpdateSpeakerName(ProjId, _speaker.Id, $" {Name}\t").Result;
+            Assert.That(result, Is.InstanceOf<BadRequestObjectResult>());
+
+            var idOfNewSpeaker = ((ObjectResult)_speakerController.CreateSpeaker(ProjId, "Ms. Other").Result).Value as string;
+            result = _speakerController.UpdateSpeakerName(ProjId, idOfNewSpeaker!, $"\t{Name}\n").Result;
+            Assert.That(result, Is.InstanceOf<BadRequestObjectResult>());
         }
 
         [Test]

Backend.Tests/Helper/SanitizationTests.cs

@@ -104,11 +104,12 @@ public void TestInvalidFileNames(string fileName)
         private static List<List<string>> _namesUnfriendlyFriendly = new()
         {
             new List<string> { "A1phaNum3ricN0Change", "A1phaNum3ricN0Change" },
+            new List<string> { "こんにちは", "こんにちは" },
             new List<string> { "RémöveOrRèpláceÄccênts", "RemoveOrReplaceAccents" },
+            new List<string> { "алтайча", "алтаича" },
             new List<string> { "math+and=currency$to<dash", "math-and-currency-to-dash" },
             new List<string> { "make spaces underscores", "make_spaces_underscores" },
             new List<string> { "(){}[]", "()()()" },
-            new List<string> { "こんにちは", "-----" },
             new List<string> { "⁇⸘¡", ",,," }
         };
         [TestCaseSource(nameof(_namesUnfriendlyFriendly))]

Backend.Tests/Mocks/SpeakerRepositoryMock.cs

@@ -77,5 +77,10 @@ public Task<ResultOfUpdate> Update(string speakerId, Speaker speaker)
             _speakers.Add(speaker.Clone());
             return Task.FromResult(ResultOfUpdate.Updated);
         }
+
+        public Task<bool> IsSpeakerNameInProject(string projectId, string name)
+        {
+            return Task.FromResult(_speakers.Any(s => s.ProjectId == projectId && s.Name == name));
+        }
     }
 }

Backend/Controllers/SpeakerController.cs

@@ -79,6 +79,21 @@ public async Task<IActionResult> GetSpeaker(string projectId, string speakerId)
             return Ok(speaker);
         }
 
+        /// <summary> Checks if given speaker name is valid for the project with given id. </summary>
+        /// <returns> null if valid; a BadRequestObjectResult if invalid. </returns>
+        private async Task<IActionResult?> CheckSpeakerName(string projectId, string name)
+        {
+            if (string.IsNullOrEmpty(name))
+            {
+                return BadRequest("projectSettings.speaker.nameEmpty");
+            }
+            if (await _speakerRepo.IsSpeakerNameInProject(projectId, name))
+            {
+                return BadRequest("projectSettings.speaker.nameTaken");
+            }
+            return null;
+        }
+
         /// <summary> Creates a <see cref="Speaker"/> for the specified projectId </summary>
         /// <returns> Id of created Speaker </returns>
         [HttpGet("create/{name}", Name = "CreateSpeaker")]
@@ -92,6 +107,14 @@ public async Task<IActionResult> CreateSpeaker(string projectId, string name)
                 return Forbid();
             }
 
+            // Ensure the new name is valid
+            name = name.Trim();
+            var nameError = await CheckSpeakerName(projectId, name);
+            if (nameError is not null)
+            {
+                return nameError;
+            }
+
             // Create speaker and return id
             var speaker = new Speaker { Name = name, ProjectId = projectId };
             return Ok((await _speakerRepo.Create(speaker)).Id);
@@ -188,6 +211,14 @@ public async Task<IActionResult> UpdateSpeakerName(string projectId, string spea
                 return NotFound(speakerId);
             }
 
+            // Ensure the new name is valid
+            name = name.Trim();
+            var nameError = await CheckSpeakerName(projectId, name);
+            if (nameError is not null)
+            {
+                return nameError;
+            }
+
             // Update name and return result with id
             speaker.Name = name;
             return await _speakerRepo.Update(speakerId, speaker) switch

Backend/Helper/FileOperations.cs

@@ -124,5 +124,15 @@ public static List<string> FindFilesWithExtension(string dir, string ext, bool r
             }
             return files;
         }
+
+        /// <summary> Like Path.ChangeExtension, but doesn't add a . for empty-string extension. </summary>
+        public static string ChangeExtension(string path, string? extension)
+        {
+            if (extension == "")
+            {
+                extension = null;
+            }
+            return Path.ChangeExtension(path, extension);
+        }
     }
 }

Backend/Helper/FileStorage.cs

@@ -99,7 +99,7 @@ public static string GenerateAvatarFilePath(string userId)
         /// <exception cref="InvalidIdException"> Throws when id invalid. </exception>
         public static string GenerateConsentFilePath(string speakerId, string? extension = null)
         {
-            var fileName = Path.ChangeExtension(Sanitization.SanitizeId(speakerId), extension);
+            var fileName = FileOperations.ChangeExtension(Sanitization.SanitizeId(speakerId), extension);
             return GenerateFilePath(ConsentDir, fileName);
         }
 

Backend/Helper/Sanitization.cs

@@ -72,7 +72,7 @@ public static string SanitizeFileName(string fileName)
 
         /// <summary>
         /// Convert a string (e.g., a project name), into one friendly to use in a path.
-        /// Uses alphanumeric and '-' '_' ',' '(' ')'.
+        /// Uses international alphanumeric and '-' '_' ',' '(' ')'.
         /// </summary>
         /// <returns> Converted string, unless length 0, then returns fallback. </returns>
         public static string MakeFriendlyForPath(string name, string fallback = "")
@@ -87,13 +87,13 @@ public static string MakeFriendlyForPath(string name, string fallback = "")
                 {
                     case UnicodeCategory.LowercaseLetter:
                     case UnicodeCategory.UppercaseLetter:
+                    case UnicodeCategory.OtherLetter:
                     case UnicodeCategory.DecimalDigitNumber:
                         stringBuilder.Append(c);
                         break;
                     case UnicodeCategory.DashPunctuation:
                     case UnicodeCategory.CurrencySymbol:
                     case UnicodeCategory.MathSymbol:
-                    case UnicodeCategory.OtherLetter:
                     case UnicodeCategory.OtherSymbol:
                         stringBuilder.Append('-');
                         break;

Backend/Interfaces/ISpeakerRepository.cs

@@ -13,5 +13,6 @@ public interface ISpeakerRepository
         Task<bool> Delete(string projectId, string speakerId);
         Task<bool> DeleteAllSpeakers(string projectId);
         Task<ResultOfUpdate> Update(string speakerId, Speaker speaker);
+        Task<bool> IsSpeakerNameInProject(string projectId, string name);
     }
 }

Backend/Repositories/SpeakerRepository.cs

@@ -37,8 +37,7 @@ public async Task<bool> DeleteAllSpeakers(string projectId)
         public async Task<Speaker?> GetSpeaker(string projectId, string speakerId)
         {
             var filterDef = new FilterDefinitionBuilder<Speaker>();
-            var filter = filterDef.And(filterDef.Eq(
-                x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId));
+            var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId));
 
             var speakerList = await _speakerDatabase.Speakers.FindAsync(filter);
             try
@@ -64,9 +63,7 @@ public async Task<Speaker> Create(Speaker speaker)
         public async Task<bool> Delete(string projectId, string speakerId)
         {
             var filterDef = new FilterDefinitionBuilder<Speaker>();
-            var filter = filterDef.And(
-                filterDef.Eq(x => x.ProjectId, projectId),
-                filterDef.Eq(x => x.Id, speakerId));
+            var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, speakerId));
 
             return (await _speakerDatabase.Speakers.DeleteOneAsync(filter)).DeletedCount > 0;
         }
@@ -88,5 +85,13 @@ public async Task<ResultOfUpdate> Update(string speakerId, Speaker speaker)
                     ? ResultOfUpdate.Updated
                     : ResultOfUpdate.NoChange;
         }
+
+        /// <summary> Check if <see cref="Speaker"/> with specified name is already in project </summary>
+        public async Task<bool> IsSpeakerNameInProject(string projectId, string name)
+        {
+            var filterDef = new FilterDefinitionBuilder<Speaker>();
+            var filter = filterDef.And(filterDef.Eq(x => x.ProjectId, projectId), filterDef.Eq(x => x.Name, name));
+            return await _speakerDatabase.Speakers.CountDocumentsAsync(filter) > 0;
+        }
     }
 }

Backend/Services/LiftService.cs

@@ -347,22 +347,39 @@ public async Task<string> LiftExport(
             // Add consent files to export directory
             foreach (var speaker in projSpeakers)
             {
-                if (speaker.Consent != ConsentType.None)
+                if (speaker.Consent == ConsentType.None)
                 {
-                    var src = FileStorage.GetConsentFilePath(speaker.Id);
-                    if (src is not null)
-                    {
-                        var dest = Path.Combine(consentDir, Path.GetFileName(src));
-                        if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase))
-                        {
-                            dest = Path.ChangeExtension(dest, ".wav");
-                            await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\"");
-                        }
-                        else
-                        {
-                            File.Copy(src, dest);
-                        }
-                    }
+                    continue;
+                }
+
+                var src = FileStorage.GetConsentFilePath(speaker.Id);
+                if (src is null || !File.Exists(src))
+                {
+                    continue;
+                };
+
+                var safeName = Sanitization.MakeFriendlyForPath(speaker.Name);
+                var fileName = safeName == "" ? Path.GetFileNameWithoutExtension(src) : safeName;
+                var fileExt = Path.GetExtension(src);
+                var convertToWav = fileExt.Equals(".webm", StringComparison.OrdinalIgnoreCase);
+                fileExt = convertToWav ? ".wav" : fileExt;
+                var dest = FileOperations.ChangeExtension(Path.Combine(consentDir, fileName), fileExt);
+
+                // Prevent collisions resulting from name sanitization
+                var duplicate = 0;
+                while (File.Exists(dest))
+                {
+                    duplicate++;
+                    dest = FileOperations.ChangeExtension(Path.Combine(consentDir, $"{fileName}{duplicate}"), fileExt);
+                }
+
+                if (convertToWav)
+                {
+                    await FFmpeg.Conversions.New().Start($"-y -i \"{src}\" \"{dest}\"");
+                }
+                else
+                {
+                    File.Copy(src, dest);
                 }
             }
 
@@ -541,11 +558,13 @@ private static async Task AddAudio(LexEntry entry, List<Pronunciation> pronuncia
         {
             foreach (var audio in pronunciations)
             {
-                var lexPhonetic = new LexPhonetic();
                 var src = FileStorage.GenerateAudioFilePath(projectId, audio.FileName);
-                var dest = Path.Combine(path, audio.FileName);
+                if (!File.Exists(src))
+                {
+                    continue;
+                };
 
-                if (!File.Exists(src)) continue;
+                var dest = Path.Combine(path, audio.FileName);
                 if (Path.GetExtension(dest).Equals(".webm", StringComparison.OrdinalIgnoreCase))
                 {
                     dest = Path.ChangeExtension(dest, ".wav");
@@ -556,16 +575,14 @@ private static async Task AddAudio(LexEntry entry, List<Pronunciation> pronuncia
                     File.Copy(src, dest, true);
                 }
 
+                var lexPhonetic = new LexPhonetic();
                 lexPhonetic.MergeIn(MultiText.Create(new LiftMultiText { { "href", dest } }));
-                // If audio has speaker, include speaker info as a pronunciation label
-                if (!audio.Protected && !string.IsNullOrEmpty(audio.SpeakerId))
+                // If audio has speaker, include speaker name as a pronunciation label
+                var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId);
+                if (speaker is not null)
                 {
-                    var speaker = projectSpeakers.Find(s => s.Id == audio.SpeakerId);
-                    if (speaker is not null)
-                    {
-                        var text = new LiftMultiText { { "en", $"Speaker #{speaker.Id}: {speaker.Name}" } };
-                        lexPhonetic.MergeIn(MultiText.Create(text));
-                    }
+                    var text = new LiftMultiText { { "en", $"Speaker: {speaker.Name}" } };
+                    lexPhonetic.MergeIn(MultiText.Create(text));
                 }
                 entry.Pronunciations.Add(lexPhonetic);
             }

public/locales/en/translation.json

@@ -201,6 +201,8 @@
       "label": "Manage speakers",
       "add": "Add a speaker",
       "enterName": "Enter the name of a new speaker",
+      "nameEmpty": "Speaker name is empty",
+      "nameTaken": "Speaker name is already taken in this project",
       "delete": "Delete this speaker",
       "edit": "Edit speaker's name",
       "consent": {