From cef428acc166cd697598ff5e99ece9931eb2a676 Mon Sep 17 00:00:00 2001 From: Pat Hartl Date: Sat, 8 Feb 2025 12:32:44 -0600 Subject: [PATCH] Optimize UserService queries, cache user roles, clean up UserService --- .../AuthenticationService.cs | 4 +- LANCommander.Server.Services/UserService.cs | 364 +++++++----------- .../Controllers/Api/DepotController.cs | 61 +-- .../Controllers/Api/GamesController.cs | 2 +- .../UI/Pages/Account/Login.cshtml.cs | 2 +- .../Users/Components/ManageRolesDialog.razor | 4 +- .../UI/Pages/Settings/Users/Index.razor | 2 +- 7 files changed, 158 insertions(+), 281 deletions(-) diff --git a/LANCommander.Server.Services/AuthenticationService.cs b/LANCommander.Server.Services/AuthenticationService.cs index 98e071f4..baa03217 100644 --- a/LANCommander.Server.Services/AuthenticationService.cs +++ b/LANCommander.Server.Services/AuthenticationService.cs @@ -63,10 +63,10 @@ namespace LANCommander.Server.Services _logger?.LogDebug("Password check for user {UserName} was successful", user.UserName); - if (_settings.Authentication.RequireApproval && !user.Approved && !await userService.IsInRoleAsync(user.UserName, RoleService.AdministratorRoleName)) + if (_settings.Authentication.RequireApproval && !user.Approved && !await userService.IsInRoleAsync(user, RoleService.AdministratorRoleName)) throw new Exception("Account must be approved by an administrator"); - var userRoles = await userService.GetRolesAsync(user.UserName); + var userRoles = await userService.GetRolesAsync(user); var authClaims = new List { diff --git a/LANCommander.Server.Services/UserService.cs b/LANCommander.Server.Services/UserService.cs index 604511ac..6af71fde 100644 --- a/LANCommander.Server.Services/UserService.cs +++ b/LANCommander.Server.Services/UserService.cs @@ -10,6 +10,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; using System.Linq.Expressions; using LANCommander.Server.Services.Exceptions; +using ZiggyCreatures.Caching.Fusion; namespace LANCommander.Server.Services { @@ -18,10 +19,12 @@ namespace LANCommander.Server.Services private readonly IdentityContext IdentityContext; private readonly CollectionService CollectionService; private readonly IMapper Mapper; + private readonly IFusionCache Cache; public UserService( ILogger logger, IMapper mapper, + IFusionCache cache, CollectionService collectionService, IdentityContextFactory identityContextFactory) : base(logger) { @@ -32,181 +35,113 @@ namespace LANCommander.Server.Services public async Task GetAsync(string userName) { - try - { - return await IdentityContext.UserManager.FindByNameAsync(userName); - } - finally - { - } + return await IdentityContext.UserManager.FindByNameAsync(userName); } public async Task GetAsync(string userName) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - return Mapper.Map(user); - } - finally - { - } + return Mapper.Map(user); } - public async Task> GetRolesAsync(string userName) + public async Task> GetRolesAsync(User user) { - try + var roles = await Cache.GetOrSetAsync($"User/{user.Id}/Roles", async _ => { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); - var roleNames = await IdentityContext.UserManager.GetRolesAsync(user); - + return await IdentityContext.RoleManager.Roles.Where(r => roleNames.Contains(r.Name)).ToListAsync(); - } - finally - { - } + }); + + return roles; } - public async Task IsInRoleAsync(string userName, string roleName) + public async Task IsInRoleAsync(User user, string roleName) + { + var roles = await GetRolesAsync(user); + + return roles.Any(r => r.Name == roleName); + } + + public async Task> GetCollectionsAsync(User user) { try { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); - - return await IdentityContext.UserManager.IsInRoleAsync(user, roleName); - } - finally - { - } - } - - public async Task> GetCollectionsAsync(Guid userId) - { - try - { - var user = await GetAsync(userId); - var roles = await GetRolesAsync(user.UserName); + var roles = await GetRolesAsync(user); var roleIds = roles.Select(r => r.Id).ToList(); - if (roles.Any(r => r.Name == RoleService.AdministratorRoleName)) + if (roles.Any(r => r.Name.Equals(RoleService.AdministratorRoleName, StringComparison.OrdinalIgnoreCase))) return await CollectionService.GetAsync(); else return await CollectionService - .Include(c => c.Roles) - .GetAsync(c => c.Roles.Any(r => roleIds.Contains(r.Id))); + .Include(c => c.Roles) + .GetAsync(c => c.Roles.Any(r => roleIds.Contains(r.Id))); } catch (Exception ex) { - _logger.LogError(ex, "Could not get user collections"); + _logger.LogError(ex, "Could not get collections for user {UserName}", user.UserName); return new List(); } } public async Task AddAsync(User user) { - try - { - var result = await IdentityContext.UserManager.CreateAsync(user); - - if (result.Succeeded) - return await IdentityContext.UserManager.FindByNameAsync(user.UserName); - else - throw new UserRegistrationException(result, "Could not create user"); - } - finally - { - } + var result = await IdentityContext.UserManager.CreateAsync(user); + + if (result.Succeeded) + return await IdentityContext.UserManager.FindByNameAsync(user.UserName); + else + throw new UserRegistrationException(result, "Could not create user"); } public async Task AddToRoleAsync(string userName, string roleName) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - await IdentityContext.UserManager.AddToRoleAsync(user, roleName); - } - finally - { - } + await IdentityContext.UserManager.AddToRoleAsync(user, roleName); } public async Task AddToRolesAsync(string userName, IEnumerable roleNames) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - var result = await IdentityContext.UserManager.AddToRolesAsync(user, roleNames); + var result = await IdentityContext.UserManager.AddToRolesAsync(user, roleNames); - if (!result.Succeeded) - throw new AddRoleException(result, "Could not add roles"); - } - finally - { - } + if (!result.Succeeded) + throw new AddRoleException(result, "Could not add roles"); } public async Task RemoveFromRole(string userName, string roleName) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - await IdentityContext.UserManager.RemoveFromRoleAsync(user, roleName); - } - finally - { - } + await IdentityContext.UserManager.RemoveFromRoleAsync(user, roleName); } public async Task CheckPassword(string userName, string password) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - return await IdentityContext.UserManager.CheckPasswordAsync(user, password); - } - finally - { - } + return await IdentityContext.UserManager.CheckPasswordAsync(user, password); } public async Task ChangePassword(string userName, string currentPassword, string newPassword) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - var result = await IdentityContext.UserManager.ChangePasswordAsync(user, currentPassword, newPassword); + var result = await IdentityContext.UserManager.ChangePasswordAsync(user, currentPassword, newPassword); - return result; - } - finally - { - } + return result; } public async Task ChangePassword(string userName, string newPassword) { - try - { - var user = await IdentityContext.UserManager.FindByNameAsync(userName); + var user = await IdentityContext.UserManager.FindByNameAsync(userName); - var token = await IdentityContext.UserManager.GeneratePasswordResetTokenAsync(user); + var token = await IdentityContext.UserManager.GeneratePasswordResetTokenAsync(user); - return await IdentityContext.UserManager.ResetPasswordAsync(user, token, newPassword); - } - catch (Exception ex) - { - throw; - } - finally - { - } + return await IdentityContext.UserManager.ResetPasswordAsync(user, token, newPassword); } public async Task SignOut() @@ -216,191 +151,152 @@ namespace LANCommander.Server.Services public async Task> GetAsync() { - try - { - return await IdentityContext.UserManager.Users.ToListAsync(); - } - finally - { - } + return await IdentityContext.UserManager.Users.ToListAsync(); } public async Task> GetAsync() { - try - { - return await IdentityContext.UserManager.Users.ProjectTo(Mapper.ConfigurationProvider).ToListAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .ProjectTo(Mapper.ConfigurationProvider) + .ToListAsync(); } public async Task GetAsync(Guid id) { - try - { - return await IdentityContext.UserManager.FindByIdAsync(id.ToString()); - } - finally - { - } + return await IdentityContext + .UserManager + .FindByIdAsync(id.ToString()); } public async Task GetAsync(Guid id) { - try - { - var user = await IdentityContext.UserManager.FindByIdAsync(id.ToString()); + var user = await IdentityContext + .UserManager + .FindByIdAsync(id.ToString()); - return Mapper.Map(user); - } - finally - { - } + return Mapper.Map(user); } public async Task> GetAsync(Expression> predicate) { - try - { - return await IdentityContext.UserManager.Users.Where(predicate).ToListAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .Where(predicate) + .ToListAsync(); } public async Task> GetAsync(Expression> predicate) { - try - { - return await IdentityContext.UserManager.Users.Where(predicate).ProjectTo(Mapper.ConfigurationProvider).ToListAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .Where(predicate) + .ProjectTo(Mapper.ConfigurationProvider) + .ToListAsync(); } public async Task FirstOrDefaultAsync(Expression> predicate) { - try - { - return await IdentityContext.UserManager.Users.FirstOrDefaultAsync(predicate); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .FirstOrDefaultAsync(predicate); } public async Task FirstOrDefaultAsync(Expression> predicate) { - try - { - return await IdentityContext.UserManager.Users.Where(predicate).ProjectTo(Mapper.ConfigurationProvider).FirstOrDefaultAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .Where(predicate) + .ProjectTo(Mapper.ConfigurationProvider) + .FirstOrDefaultAsync(); } public async Task FirstOrDefaultAsync(Expression> predicate, Expression> orderKeySelector) { - try - { - return await IdentityContext.UserManager.Users.Where(predicate).OrderBy(orderKeySelector).FirstOrDefaultAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .Where(predicate) + .OrderBy(orderKeySelector) + .FirstOrDefaultAsync(); } public async Task FirstOrDefaultAsync(Expression> predicate, Expression> orderKeySelector) { - try - { - return await IdentityContext.UserManager.Users.Where(predicate).ProjectTo(Mapper.ConfigurationProvider).OrderBy(orderKeySelector).FirstOrDefaultAsync(); - } - finally - { - } + return await IdentityContext + .UserManager + .Users + .Where(predicate) + .ProjectTo(Mapper.ConfigurationProvider) + .OrderBy(orderKeySelector) + .FirstOrDefaultAsync(); } public async Task ExistsAsync(Guid id) { - try - { - var user = await IdentityContext.UserManager.FindByIdAsync(id.ToString()); + var user = await IdentityContext + .UserManager + .FindByIdAsync(id.ToString()); - return user != null; - } - finally - { - } + return user != null; } public async Task> AddMissingAsync(Expression> predicate, User entity) { - try + var result = new ExistingEntityResult(); + + var user = await IdentityContext + .UserManager + .Users + .FirstOrDefaultAsync(predicate); + + if (user == null) { - var result = new ExistingEntityResult(); + await IdentityContext.UserManager.CreateAsync(entity); - var user = await IdentityContext.UserManager.Users.FirstOrDefaultAsync(predicate); - - if (user == null) - { - await IdentityContext.UserManager.CreateAsync(entity); - - result.Existing = false; - result.Value = await IdentityContext.UserManager.FindByNameAsync(user.UserName); - } - else - { - result.Existing = true; - result.Value = user; - } - - return result; + result.Existing = false; + result.Value = await IdentityContext.UserManager.FindByNameAsync(user.UserName); } - finally + else { + result.Existing = true; + result.Value = user; } + + return result; } public async Task UpdateAsync(User entity) { - try - { - var user = await IdentityContext.UserManager.FindByIdAsync(entity.Id.ToString()); + var user = await IdentityContext + .UserManager + .FindByIdAsync(entity.Id.ToString()); - user.UserName = entity.UserName; - user.PhoneNumber = entity.PhoneNumber; - user.Email = entity.Email; - user.TwoFactorEnabled = entity.TwoFactorEnabled; - user.Alias = entity.Alias; - user.Approved = entity.Approved; - user.ApprovedOn = entity.ApprovedOn; + user.UserName = entity.UserName; + user.PhoneNumber = entity.PhoneNumber; + user.Email = entity.Email; + user.TwoFactorEnabled = entity.TwoFactorEnabled; + user.Alias = entity.Alias; + user.Approved = entity.Approved; + user.ApprovedOn = entity.ApprovedOn; - await IdentityContext.UserManager.UpdateAsync(user); + await IdentityContext.UserManager.UpdateAsync(user); - return user; - } - finally - { - } + return user; } public async Task DeleteAsync(User entity) { - try - { - var user = await IdentityContext.UserManager.FindByIdAsync(entity.Id.ToString()); + var user = await IdentityContext + .UserManager + .FindByIdAsync(entity.Id.ToString()); - await IdentityContext.UserManager.DeleteAsync(user); - } - finally - { - } + await IdentityContext.UserManager.DeleteAsync(user); } public IBaseDatabaseService Include(Expression> includeExpression) diff --git a/LANCommander.Server/Controllers/Api/DepotController.cs b/LANCommander.Server/Controllers/Api/DepotController.cs index 41f1df47..d7bf76a4 100644 --- a/LANCommander.Server/Controllers/Api/DepotController.cs +++ b/LANCommander.Server/Controllers/Api/DepotController.cs @@ -29,7 +29,6 @@ namespace LANCommander.Server.Controllers.Api private readonly TagService TagService; private readonly LibraryService LibraryService; private readonly UserService UserService; - private readonly DatabaseContext DatabaseContext; public DepotController( ILogger logger, @@ -43,8 +42,7 @@ namespace LANCommander.Server.Controllers.Api PlatformService platformService, TagService tagService, LibraryService libraryService, - UserService userService, - DatabaseContext databaseContext) : base(logger) + UserService userService) : base(logger) { Mapper = mapper; Cache = cache; @@ -57,7 +55,6 @@ namespace LANCommander.Server.Controllers.Api TagService = tagService; LibraryService = libraryService; UserService = userService; - DatabaseContext = databaseContext; } [HttpGet] @@ -68,46 +65,29 @@ namespace LANCommander.Server.Controllers.Api var results = await Cache.GetOrSetAsync("Depot/Results", async _ => { - var results = new SDK.Models.DepotResults(); - - results.Games = await GameService.Query(q => + var results = new SDK.Models.DepotResults() { - return q.AsNoTracking(); - }).GetAsync(); - - results.Collections = await CollectionService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); - - results.Companies = await CompanyService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); - - results.Engines = await EngineService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); - - results.Genres = await GenreService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); - - results.Platforms = await PlatformService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); - - results.Tags = await TagService.Query(q => - { - return q.AsNoTracking(); - }).GetAsync(); + Games = await GameService.AsNoTracking().GetAsync(), + Collections = await CollectionService.AsNoTracking().GetAsync(), + Companies = await CompanyService.AsNoTracking().GetAsync(), + Engines = await EngineService.AsNoTracking().GetAsync(), + Genres = await GenreService.AsNoTracking().GetAsync(), + Platforms = await PlatformService.AsNoTracking().GetAsync(), + Tags = await TagService.AsNoTracking().GetAsync(), + }; return results; }, TimeSpan.MaxValue); + var collections = await UserService.GetCollectionsAsync(user); + + results.Games = results + .Games + .Where(g => + g.Collections.Any(gc => + collections.Any(c => c.Id == gc.Id))) + .ToList(); + foreach (var game in results.Games) { game.InLibrary = library.Games.Any(g => g.Id == game.Id); @@ -119,6 +99,8 @@ namespace LANCommander.Server.Controllers.Api [HttpGet("Games/{id}")] public async Task GetGameAsync(Guid id) { + var user = await UserService.GetAsync(User?.Identity?.Name); + var game = await Cache.GetOrSetAsync($"Depot/Games/{id}", async _ => { return await GameService.Query(q => @@ -145,7 +127,6 @@ namespace LANCommander.Server.Controllers.Api .GetAsync(id); }); - var user = await UserService.GetAsync(User?.Identity?.Name); var library = await LibraryService .AsNoTracking() .Include(l => l.Games) diff --git a/LANCommander.Server/Controllers/Api/GamesController.cs b/LANCommander.Server/Controllers/Api/GamesController.cs index 61ca0711..f313495c 100644 --- a/LANCommander.Server/Controllers/Api/GamesController.cs +++ b/LANCommander.Server/Controllers/Api/GamesController.cs @@ -70,7 +70,7 @@ namespace LANCommander.Server.Controllers.Api if (Settings.Roles.RestrictGamesByCollection && !User.IsInRole(RoleService.AdministratorRoleName)) { - var roles = await UserService.GetRolesAsync(User?.Identity.Name); + var roles = await UserService.GetRolesAsync(user); var accessibleCollectionIds = roles.SelectMany(r => r.Collections.Select(c => c.Id)).Distinct(); diff --git a/LANCommander.Server/UI/Pages/Account/Login.cshtml.cs b/LANCommander.Server/UI/Pages/Account/Login.cshtml.cs index 9e3bbe71..df5db562 100644 --- a/LANCommander.Server/UI/Pages/Account/Login.cshtml.cs +++ b/LANCommander.Server/UI/Pages/Account/Login.cshtml.cs @@ -138,7 +138,7 @@ namespace LANCommander.Server.UI.Pages.Account { var user = await UserService.GetAsync(Model.Username); - if (user != null && !user.Approved && !(await UserService.IsInRoleAsync(user.UserName, RoleService.AdministratorRoleName))) + if (user != null && !user.Approved && !(await UserService.IsInRoleAsync(user, RoleService.AdministratorRoleName))) { ModelState.AddModelError(string.Empty, "Your account must be approved by an administrator."); return Page(); diff --git a/LANCommander.Server/UI/Pages/Settings/Users/Components/ManageRolesDialog.razor b/LANCommander.Server/UI/Pages/Settings/Users/Components/ManageRolesDialog.razor index b3f4137a..e7e3becd 100644 --- a/LANCommander.Server/UI/Pages/Settings/Users/Components/ManageRolesDialog.razor +++ b/LANCommander.Server/UI/Pages/Settings/Users/Components/ManageRolesDialog.razor @@ -38,7 +38,7 @@ User = await UserService.GetAsync(Options); Roles = await RoleService.GetAsync(); - var currentRoles = await UserService.GetRolesAsync(User.UserName); + var currentRoles = await UserService.GetRolesAsync(User); SelectedRoles = Roles.Where(r => currentRoles.Any(cr => r.Name == cr.Name)).ToList(); } @@ -54,7 +54,7 @@ try { - var currentRoles = await UserService.GetRolesAsync(User.UserName); + var currentRoles = await UserService.GetRolesAsync(User); await UserService.AddToRolesAsync(User.UserName, SelectedRoles.Select(r => r.Name).Where(r => !currentRoles.Any(cr => cr.Name == r))); diff --git a/LANCommander.Server/UI/Pages/Settings/Users/Index.razor b/LANCommander.Server/UI/Pages/Settings/Users/Index.razor index e0c01c22..84e3b196 100644 --- a/LANCommander.Server/UI/Pages/Settings/Users/Index.razor +++ b/LANCommander.Server/UI/Pages/Settings/Users/Index.razor @@ -82,7 +82,7 @@ if (Directory.Exists(savePath)) saveSize = new DirectoryInfo(savePath).EnumerateFiles("*", SearchOption.AllDirectories).Sum(f => f.Length); - var roles = await UserService.GetRolesAsync(user.UserName); + var roles = await UserService.GetRolesAsync(user); UserList.Add(new UserViewModel() {