Follow bunny advice

Author: imnasnainaec

Backend.Tests/Controllers/AudioControllerTests.cs

@@ -163,15 +163,12 @@ public void TestDeleteAudioFileInvalidArguments()
         [Test]
         public void TestDeleteAudioFileNoWordWithAudio()
         {
-            var result = _audioController.DeleteAudioFile(_projId, "not-a-word", _file.FileName).Result;
-            Assert.That(result, Is.InstanceOf<NotFoundObjectResult>());
+            var result1 = _audioController.DeleteAudioFile(_projId, "not-a-word", _file.FileName).Result;
+            Assert.That(result1, Is.InstanceOf<NotFoundObjectResult>());
 
             var wordId = _wordRepo.Create(Util.RandomWord(_projId)).Result.Id;
-            var ex = Assert.Throws<AggregateException>(() =>
-            {
-                _ = _audioController.DeleteAudioFile(_projId, wordId, _file.FileName).Result;
-            });
-            Assert.That(ex?.InnerException, Is.InstanceOf<ArgumentException>());
+            var result2 = _audioController.DeleteAudioFile(_projId, wordId, _file.FileName).Result;
+            Assert.That(result2, Is.InstanceOf<NotFoundObjectResult>());
         }
 
         [Test]

Backend.Tests/Mocks/WordRepositoryMock.cs

@@ -233,7 +233,7 @@ public Task<bool> RestoreFrontier(string projectId, string wordId)
             return await UpdateFrontier(word, createIfNotFound: false, modifyUpdatedWord);
         }
 
-        public async Task<List<Word>?> ReplaceFrontier(string projectId, List<Word> newWords,
+        public async Task<List<Word>> ReplaceFrontier(string projectId, List<Word> newWords,
             List<string> idsToDelete, Action<Word, Word?> modifyUpdatedWord, Action<Word> modifyDeletedWord)
         {
             if (newWords.Any(w => w.ProjectId != projectId))

Backend.Tests/Services/WordServiceTests.cs

@@ -99,9 +99,8 @@ public void TestDeleteAudioBadInput()
             Assert.That(_wordService.DeleteAudio("non-proj-id", UserId, wordInFrontier.Id, fileName).Result, Is.Null);
             Assert.That(_wordService.DeleteAudio(ProjId, UserId, "non-word-id", fileName).Result, Is.Null);
 
-            var ex = Assert.Throws<AggregateException>(
-                () => _wordService.DeleteAudio(ProjId, UserId, wordInFrontier.Id, "non-file-name").Wait());
-            Assert.That(ex?.InnerException, Is.InstanceOf<ArgumentException>());
+            var result = _wordService.DeleteAudio(ProjId, UserId, wordInFrontier.Id, "non-file-name").Result;
+            Assert.That(result, Is.Null);
         }
 
         [Test]
@@ -197,9 +196,9 @@ public void TestRestoreFrontierWordAlreadyInFrontierThrows()
             var wordInFrontier = _wordRepo.Create(new Word { ProjectId = ProjId }).Result;
             Assert.That(_wordRepo.GetAllFrontier(ProjId).Result, Has.Count.EqualTo(1));
 
-            Assert.That(
-                () => _wordService.RestoreFrontierWord(ProjId, wordInFrontier.Id).Wait(),
-                Throws.TypeOf<AggregateException>());
+            var ex = Assert.Throws<AggregateException>(
+                () => _wordService.RestoreFrontierWord(ProjId, wordInFrontier.Id).Wait());
+            Assert.That(ex?.InnerException, Is.InstanceOf<ArgumentException>());
         }
 
         [Test]

Backend/Contexts/MongoDbContext.cs

@@ -33,8 +33,16 @@ public MongoDbContext(IOptions<Startup.Settings> options)
     public async Task<IMongoTransaction> BeginTransaction()
     {
         var session = await Db.Client.StartSessionAsync();
-        session.StartTransaction();
-        return new MongoTransactionWrapper(session);
+        try
+        {
+            session.StartTransaction();
+            return new MongoTransactionWrapper(session);
+        }
+        catch
+        {
+            session.Dispose();
+            throw;
+        }
     }
 
     /// <summary>

Backend/Interfaces/IWordRepository.cs

@@ -25,7 +25,7 @@ public interface IWordRepository
         Task<bool> RestoreFrontier(string projectId, string wordId);
         Task<Word?> UpdateFrontier(string projectId, string wordId, Action<Word> modifyUpdatedWord);
         Task<Word?> UpdateFrontier(Word word, Action<Word, Word?> modifyUpdatedWord);
-        Task<List<Word>?> ReplaceFrontier(string projectId, List<Word> newWords, List<string> idsToDelete,
+        Task<List<Word>> ReplaceFrontier(string projectId, List<Word> newWords, List<string> idsToDelete,
             Action<Word, Word?> modifyUpdatedWord, Action<Word> modifyDeletedWord);
         Task<bool> RevertReplaceFrontier(string projectId, List<string> idsToRestore, List<string> idsToDelete,
             Action<Word> modifyDeletedWord);

Backend/Interfaces/IWordService.cs

@@ -13,7 +13,7 @@ public interface IWordService
         Task<bool> RestoreFrontierWord(string projectId, string wordId);
         Task<Word?> Update(string userId, Word word);
         Task<string?> FindContainingWord(Word word);
-        Task<List<Word>?> MergeReplaceFrontier(
+        Task<List<Word>> MergeReplaceFrontier(
             string projectId, string userId, List<Word> parents, List<string> idsToDelete);
         Task<bool> RevertMergeReplaceFrontier(
             string projectId, string userId, List<string> idsToRestore, List<string> idsToDelete);

Backend/Repositories/WordRepository.cs

@@ -124,7 +124,7 @@ public async Task<List<Word>> Create(List<Word> words)
                 OtelService.StartActivityWithTag(otelTagName, "creating words in WordsCollection and Frontier");
 
             return words.Count == 0
-                ? []
+                ? words
                 : await _dbContext.ExecuteInTransaction(async s => await CreateWithSession(s, words));
         }
 
@@ -335,15 +335,13 @@ public async Task<bool> RestoreFrontier(string projectId, string wordId)
         /// <param name="modifyDeletedWord">
         /// Action applied to words deleted from Frontier before inserting into WordsCollection.
         /// </param>
-        /// <returns>
-        /// The replacement words when successful, an empty list if no work is needed, or null if replacement fails.
-        /// </returns>
-        public async Task<List<Word>?> ReplaceFrontier(string projectId, List<Word> newWords,
+        /// <returns>The replacement words when successful, an empty list if no work is needed.</returns>
+        public async Task<List<Word>> ReplaceFrontier(string projectId, List<Word> newWords,
             List<string> idsToDelete, Action<Word, Word?> modifyUpdatedWord, Action<Word> modifyDeletedWord)
         {
             return (newWords.Count == 0 && idsToDelete.Count == 0)
-                ? []
-                : await _dbContext.ExecuteInTransactionAllowNull(async s => await ReplaceFrontierWithSession(
+                ? newWords
+                : await _dbContext.ExecuteInTransaction(async s => await ReplaceFrontierWithSession(
                     s, projectId, newWords, idsToDelete, modifyUpdatedWord, modifyDeletedWord));
         }
 
@@ -469,8 +467,9 @@ private async Task<List<Word>> CreateWithSession(IClientSessionHandle session, L
         /// <param name="modifyDeletedWord">Action applied on deleted Frontier words added to WordsCollection.</param>
         /// <returns>True when all requested restores succeed; otherwise false.</returns>
         /// <exception cref="ArgumentException">Thrown when restore and delete id sets are not disjoint.</exception>
-        private async Task<bool> RevertReplaceFrontierWithSession(IClientSessionHandle session, string projectId,
-            IEnumerable<string> idsToRestore, IEnumerable<string> idsToDelete, Action<Word> modifyDeletedWord)
+        private async Task<bool> RevertReplaceFrontierWithSession(IClientSessionHandle session,
+            string projectId, IEnumerable<string> idsToRestore, IEnumerable<string> idsToDelete,
+            Action<Word> modifyDeletedWord)
         {
             var restoreSet = idsToRestore.ToHashSet(); // Remove duplicates
             if (restoreSet.Intersect(idsToDelete).Any())
@@ -498,9 +497,9 @@ private async Task<bool> RevertReplaceFrontierWithSession(IClientSessionHandle s
         /// <param name="oldWordIds">Ids of Frontier words that will be replaced or deleted.</param>
         /// <param name="modifyUpdatedWord">Action applied when building each replacement word.</param>
         /// <param name="modifyDeletedWord">Action applied on deleted Frontier words added to WordsCollection.</param>
-        /// <returns>The replacement words, or null if a required update fails.</returns>
+        /// <returns>The replaced words.</returns>
         /// <exception cref="ArgumentException">Thrown when a replacement word has a different project id.</exception>
-        private async Task<List<Word>?> ReplaceFrontierWithSession(IClientSessionHandle session,
+        private async Task<List<Word>> ReplaceFrontierWithSession(IClientSessionHandle session,
             string projectId, List<Word> newWords, IEnumerable<string> oldWordIds,
             Action<Word, Word?> modifyUpdatedWord, Action<Word> modifyDeletedWord)
         {
@@ -509,15 +508,14 @@ private async Task<bool> RevertReplaceFrontierWithSession(IClientSessionHandle s
                 throw new ArgumentException("All new words must have the specified projectId");
             }
 
-            var oldIdSet = oldWordIds.ToHashSet(); // Remove duplicates
+            var oldIdSet = oldWordIds.ToHashSet(); // Remove duplicates and allow easy removal for each update.
 
             foreach (var word in newWords)
             {
+                // Remove the id from the old ids (if present) before the word is updated and given a new id.
                 oldIdSet.Remove(word.Id);
-                if (await UpdateFrontierWithSession(session, word, createIfNotFound: true, modifyUpdatedWord) is null)
-                {
-                    return null;
-                }
+                // `createIfNotFound: true` so the word is created even if the id isn't in the Frontier.
+                await UpdateFrontierWithSession(session, word, createIfNotFound: true, modifyUpdatedWord);
             }
 
             // Delete any remaining old words that weren't updated with a new word

Backend/Services/MergeService.cs

@@ -139,8 +139,7 @@ public async Task<List<Word>> Merge(string projectId, string userId, List<MergeW
 
             // Consolidate children ids
             var childrenIds = mergeWordsList.SelectMany(m => m.Children.Select(c => c.SrcWordId)).ToHashSet();
-            return await _wordService.MergeReplaceFrontier(
-                projectId, userId, parents.ToList(), childrenIds.ToList()) ?? [];
+            return await _wordService.MergeReplaceFrontier(projectId, userId, parents.ToList(), childrenIds.ToList());
         }
 
         /// <summary> Undo merge </summary>

Backend/Services/WordService.cs

@@ -151,7 +151,14 @@ public async Task<Word> Create(string userId, Word word)
         {
             using var activity = OtelService.StartActivityWithTag(otelTagName, "deleting an audio");
 
-            return await _wordRepo.UpdateFrontier(projectId, wordId, CreateDeleteAudioAction(userId, fileName));
+            try
+            {
+                return await _wordRepo.UpdateFrontier(projectId, wordId, CreateDeleteAudioAction(userId, fileName));
+            }
+            catch (ArgumentException)
+            {
+                return null;
+            }
         }
 
         /// <summary> Removes word from Frontier and adds a Deleted copy in the words collection </summary>
@@ -208,8 +215,8 @@ public async Task<bool> RestoreFrontierWord(string projectId, string wordId)
         /// <param name="userId">Id of the user performing the merge.</param>
         /// <param name="parents">Parent words to create or use as replacements.</param>
         /// <param name="idsToDelete">Ids of merge children to delete from Frontier.</param>
-        /// <returns>The updated parent words, or null if replacement fails.</returns>
-        public async Task<List<Word>?> MergeReplaceFrontier(
+        /// <returns>The updated parent words.</returns>
+        public async Task<List<Word>> MergeReplaceFrontier(
             string projectId, string userId, List<Word> parents, List<string> idsToDelete)
         {
             using var activity = OtelService.StartActivityWithTag(otelTagName, "replacing frontier words for merge");