Implement CodeQL recommendations

sillsdev · Apr 8, 2024 · f02d3dc · f02d3dc
1 parent b4f3f3f
commit f02d3dc
Show file tree

Hide file tree

Showing 16 changed files with 84 additions and 120 deletions.
diff --git a/Backend.Tests/Mocks/PermissionServiceMock.cs b/Backend.Tests/Mocks/PermissionServiceMock.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using BackendFramework.Interfaces;
 using BackendFramework.Models;

diff --git a/Backend.Tests/Mocks/UserRepositoryMock.cs b/Backend.Tests/Mocks/UserRepositoryMock.cs
@@ -1,7 +1,6 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using BackendFramework.Helper;
 using BackendFramework.Interfaces;

diff --git a/Backend/Controllers/InviteController.cs b/Backend/Controllers/InviteController.cs
@@ -92,7 +92,7 @@ public async Task<IActionResult> ValidateToken(string projectId, string token)
                 if (user.Email == tokenObj.Email)
                 {
                     currentUser = user;
-                    if (!user.ProjectRoles.ContainsKey(projectId))
+                    if (!user.ProjectRoles.TryGetValue(projectId, out var _roleId))
                     {
                         isUserRegisteredAndNotInProject = true;
                     }

diff --git a/Backend/Controllers/ProjectController.cs b/Backend/Controllers/ProjectController.cs
@@ -54,7 +54,7 @@ public async Task<IActionResult> GetAllProjectUsers(string projectId)
             }
 
             var allUsers = await _userRepo.GetAllUsers();
-            var projectUsers = allUsers.FindAll(user => user.ProjectRoles.ContainsKey(projectId));
+            var projectUsers = allUsers.FindAll(user => user.ProjectRoles.TryGetValue(projectId, out var _roleId));
 
             return Ok(projectUsers);
         }

diff --git a/Backend/Controllers/UserRoleController.cs b/Backend/Controllers/UserRoleController.cs
@@ -108,11 +108,11 @@ public async Task<IActionResult> GetCurrentPermissions(string projectId)
                 return NotFound($"user: {userId}");
             }
 
-            if (!user.ProjectRoles.ContainsKey(projectId))
+            if (!user.ProjectRoles.TryGetValue(projectId, out var roleId))
             {
                 return Ok(new List<Permission>());
             }
-            var userRole = await _userRoleRepo.GetUserRole(projectId, user.ProjectRoles[projectId]);
+            var userRole = await _userRoleRepo.GetUserRole(projectId, roleId);
             if (userRole is null)
             {
                 return Ok(new List<Permission>());
@@ -231,12 +231,7 @@ public async Task<IActionResult> UpdateUserRole(
                 return NotFound(userId);
             }
 
-            string userRoleId;
-            if (changeUser.ProjectRoles.ContainsKey(projectId))
-            {
-                userRoleId = changeUser.ProjectRoles[projectId];
-            }
-            else
+            if (!changeUser.ProjectRoles.TryGetValue(projectId, out var userRoleId))
             {
                 // Generate the userRole
                 var usersRole = new UserRole { ProjectId = projectId };

diff --git a/Backend/Helper/FileOperations.cs b/Backend/Helper/FileOperations.cs
@@ -3,7 +3,6 @@
 using System.IO;
 using System.IO.Compression;
 using System.Linq;
-using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 

diff --git a/Backend/Helper/FileStorage.cs b/Backend/Helper/FileStorage.cs
@@ -2,7 +2,6 @@
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
-using System.Runtime.Serialization;
 
 namespace BackendFramework.Helper
 {

diff --git a/Backend/Helper/Language.cs b/Backend/Helper/Language.cs
@@ -43,7 +43,7 @@ public static List<WritingSystem> ConvertLangTagsToWritingSystems(IEnumerable<st
         /// </summary>
         public static List<WritingSystem> GetWritingSystems(string dirPath)
         {
-            if (!Directory.GetFiles(dirPath, "*.ldml").Any())
+            if (Directory.GetFiles(dirPath, "*.ldml").Length == 0)
             {
                 dirPath = FileStorage.GenerateWritingsSystemsSubdirPath(dirPath);
             }

diff --git a/Backend/Helper/LiftHelper.cs b/Backend/Helper/LiftHelper.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using System.Runtime.Serialization;
 using BackendFramework.Models;
 using SIL.Lift.Parsing;
 

diff --git a/Backend/Helper/Sanitization.cs b/Backend/Helper/Sanitization.cs
@@ -3,7 +3,6 @@
 using System.Collections.Immutable;
 using System.Globalization;
 using System.Linq;
-using System.Runtime.Serialization;
 using System.Text;
 
 namespace BackendFramework.Helper

diff --git a/Backend/Services/InviteService.cs b/Backend/Services/InviteService.cs
@@ -1,5 +1,4 @@
 using System;
-using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using BackendFramework.Interfaces;
 using BackendFramework.Models;
@@ -68,11 +67,8 @@ public async Task<bool> RemoveTokenAndCreateUserRole(Project project, User user,
                 user.ProjectRoles.Add(project.Id, userRole.Id);
                 await _userRepo.Update(user.Id, user);
                 // Generate the JWT based on those new userRoles
-                var updatedUser = await _permissionService.MakeJwt(user);
-                if (updatedUser is null)
-                {
-                    throw new PermissionService.InvalidJwtTokenException("Unable to generate JWT.");
-                }
+                var updatedUser = await _permissionService.MakeJwt(user)
+                    ?? throw new PermissionService.InvalidJwtTokenException("Unable to generate JWT.");
 
                 await _userRepo.Update(updatedUser.Id, updatedUser);
 

diff --git a/Backend/Services/LiftService.cs b/Backend/Services/LiftService.cs
@@ -4,7 +4,6 @@
 using System.IO;
 using System.IO.Compression;
 using System.Linq;
-using System.Runtime.Serialization;
 using System.Security;
 using System.Threading.Tasks;
 using System.Xml;
@@ -33,7 +32,7 @@ public CombineLiftWriter(string path, ByteOrderStyle byteOrderStyle) : base(path
         protected override void InsertPronunciationIfNeeded(
             LexEntry entry, List<string> propertiesAlreadyOutput)
         {
-            if (entry.Pronunciations.FirstOrDefault() is not null && entry.Pronunciations.First().Forms.Any())
+            if (entry.Pronunciations.Count != 0 && entry.Pronunciations.First().Forms.Length != 0)
             {
                 foreach (var phonetic in entry.Pronunciations)
                 {
@@ -141,11 +140,8 @@ public void SetExportInProgress(string userId, bool isInProgress)
         /// <summary> Query whether user has an in-progress export. </summary>
         public bool IsExportInProgress(string userId)
         {
-            if (!_liftExports.ContainsKey(userId))
-            {
-                return false;
-            }
-            return _liftExports[userId] == InProgress;
+            _liftExports.TryGetValue(userId, out var exportPath);
+            return exportPath == InProgress;
         }
 
         /// <summary> Store filePath for a user's Lift export. </summary>
@@ -159,12 +155,8 @@ public void StoreExport(string userId, string filePath)
         /// <returns> Path to the Lift file on disk. </returns>
         public string? RetrieveExport(string userId)
         {
-            if (!_liftExports.ContainsKey(userId) || _liftExports[userId] == InProgress)
-            {
-                return null;
-            }
-
-            return _liftExports[userId];
+            _liftExports.TryGetValue(userId, out var exportPath);
+            return exportPath == InProgress ? null : exportPath;
         }
 
         /// <summary> Delete a stored Lift export path and its file on disk. </summary>
@@ -190,12 +182,8 @@ public void StoreImport(string userId, string filePath)
         /// <returns> Path to the Lift file on disk. </returns>
         public string? RetrieveImport(string userId)
         {
-            if (!_liftImports.ContainsKey(userId))
-            {
-                return null;
-            }
-
-            return _liftImports[userId];
+            _liftImports.TryGetValue(userId, out var importPath);
+            return importPath;
         }
 
         /// <summary> Delete a stored Lift import path and its file on disk. </summary>
@@ -214,7 +202,7 @@ public bool DeleteImport(string userId)
         /// <returns> A bool indicating whether a character set was added to the project. </returns>
         public async Task<bool> LdmlImport(string dirPath, IProjectRepository projRepo, Project project)
         {
-            if (!Directory.GetFiles(dirPath, "*.ldml").Any())
+            if (Directory.GetFiles(dirPath, "*.ldml").Length == 0)
             {
                 dirPath = FileStorage.GenerateWritingsSystemsSubdirPath(dirPath);
             }
@@ -478,11 +466,11 @@ private static void AddSenses(LexEntry entry, Word wordEntry, Dictionary<string,
                 var defDict = new Dictionary<string, string>();
                 foreach (var def in currentSense.Definitions)
                 {
-                    if (defDict.ContainsKey(def.Language))
+                    if (defDict.TryGetValue(def.Language, out var defText))
                     {
                         // This is an unexpected situation but rather than crashing or losing data we
                         // will just append extra definitions for the language with a separator.
-                        defDict[def.Language] = $"{defDict[def.Language]}{sep}{def.Text}";
+                        defDict[def.Language] = $"{defText}{sep}{def.Text}";
                     }
                     else
                     {
@@ -492,11 +480,11 @@ private static void AddSenses(LexEntry entry, Word wordEntry, Dictionary<string,
                 var glossDict = new Dictionary<string, string>();
                 foreach (var gloss in currentSense.Glosses)
                 {
-                    if (glossDict.ContainsKey(gloss.Language))
+                    if (glossDict.TryGetValue(gloss.Language, out var glossDef))
                     {
                         // This is an unexpected situation but rather than crashing or losing data we
                         // will just append extra definitions for the language with a separator.
-                        glossDict[gloss.Language] = $"{glossDict[gloss.Language]}{sep}{gloss.Def}";
+                        glossDict[gloss.Language] = $"{glossDef}{sep}{gloss.Def}";
                     }
                     else
                     {

diff --git a/Backend/Services/MergeService.cs b/Backend/Services/MergeService.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
-using System.Runtime.Serialization;
 using BackendFramework.Helper;
 using BackendFramework.Interfaces;
 using BackendFramework.Models;

diff --git a/Backend/Services/PermissionService.cs b/Backend/Services/PermissionService.cs
@@ -2,7 +2,6 @@
 using System.Collections.Generic;
 using System.IdentityModel.Tokens.Jwt;
 using System.Linq;
-using System.Runtime.Serialization;
 using System.Security.Claims;
 using System.Text;
 using System.Text.Json;