From 92a6c81daf7ac82f4ad9bfc95fcea617607452b0 Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Thu, 3 Dec 2020 14:52:12 +0100 Subject: [PATCH 1/5] Add Collaborators by performing for each loop instead of joining tables Joining the tables by using Include() caused the query to take sometimes up to 10 times as long. This was an issue as it caused drastic slow down when retrieving all projects, retrieving project detail or when searching for projects. --- Repositories/ProjectRepository.cs | 324 +++++++++++++++++------------- 1 file changed, 185 insertions(+), 139 deletions(-) diff --git a/Repositories/ProjectRepository.cs b/Repositories/ProjectRepository.cs index e8498ae9..96acb34b 100644 --- a/Repositories/ProjectRepository.cs +++ b/Repositories/ProjectRepository.cs @@ -16,7 +16,6 @@ */ using Microsoft.EntityFrameworkCore; -using Microsoft.EntityFrameworkCore.Query; using Models; using Models.Defaults; using Repositories.Base; @@ -29,6 +28,7 @@ namespace Repositories { + public interface IProjectRepository : IRepository { @@ -38,7 +38,7 @@ Task> GetAllWithUsersAndCollaboratorsAsync( Expression> orderBy = null, bool orderByAsc = true, bool? highlighted = null - ); + ); Task CountAsync(bool? highlighted = null); @@ -54,6 +54,7 @@ Task> SearchAsync( Task SearchCountAsync(string query, bool? highlighted = null); Task FindWithUserAndCollaboratorsAsync(int id); + } public class ProjectRepository : Repository, IProjectRepository @@ -62,106 +63,28 @@ public class ProjectRepository : Repository, IProjectRepository public ProjectRepository(DbContext dbContext) : base(dbContext) { } /// - /// Redact user email from the Project if isPublic setting is set to false - /// - /// The project. - /// - /// Project with possibly redacted email depending on setting - /// - private Project RedactUser(Project project) - { - if(project == null) return null; - - if(project?.User?.IsPublic == false) - { - project.User.Email = Defaults.Privacy.RedactedEmail; - } - return project; - } - - /// - /// Redact user email from the Projects in the list. - /// Email will only be redacted if isPublic setting is set to false. - /// - /// The projects. - /// - /// List of Projects with possibly redacted email depending on setting - /// - private List RedactUser(List projects) - { - for(int i = 0; i < projects.Count; i++) - { - projects[i] = RedactUser(projects[i]); - } - return projects; - } - - /// - /// Find the project async by project id + /// Find the project async by project id /// /// The identifier. /// - /// Project with possibly redacted email + /// Project with possibly redacted email /// public override async Task FindAsync(int id) { Project project = await GetDbSet() - .Where(s => s.Id == id) - .Include(p => p.Collaborators) - .Include(p => p.CallToAction) - .SingleOrDefaultAsync(); + .Where(s => s.Id == id) + .Include(p => p.ProjectIcon) + .Include(p => p.CallToAction) + .SingleOrDefaultAsync(); - return RedactUser(project); - } - - /// - /// Apply query parameters and find project based on these filters - /// - /// The linq queryable object. - /// The amount of objects to skip. - /// The amount of objects to take. - /// The order by expression. - /// if set to true [order by asc]. - /// Boolean if the project should show highlighted. - /// - /// IQueryable Projects based on the given filters - /// - private IQueryable ApplyFilters( - IQueryable queryable, - int? skip, - int? take, - Expression> orderBy, - bool orderByAsc, - bool? highlighted - ) - { - if(highlighted.HasValue) + if(project != null) { - IEnumerable highlightedQueryable = DbContext.Set() - .Where(h => h.StartDate <= DateTime.Now || - h.StartDate == null) - .Where(h => h.EndDate >= DateTime.Now || - h.EndDate == null) - .Select(h => h.ProjectId) - .ToList(); - - queryable = queryable.Where(p => highlightedQueryable.Contains(p.Id) == highlighted.Value); + project.Collaborators = await GetDbSet() + .Where(p => p.ProjectId == project.Id) + .ToListAsync(); } - if(orderBy != null) - { - if(orderByAsc) - { - queryable = queryable.OrderBy(orderBy); - } else - { - queryable = queryable.OrderByDescending(orderBy); - } - } - - if(skip.HasValue) queryable = queryable.Skip(skip.Value); - if(take.HasValue) queryable = queryable.Take(take.Value); - return queryable; + return RedactUser(project); } /// @@ -179,28 +102,35 @@ public virtual async Task> GetAllWithUsersAndCollaboratorsAsync( Expression> orderBy = null, bool orderByAsc = true, bool? highlighted = null - ) + ) { - IQueryable queryable = DbSet - .Include(p => p.User) - .Include(p => p.ProjectIcon) - .Include(p => p.CallToAction) - .Include(p => p.Collaborators); + IQueryable queryableProjects = GetDbSet() + .Include(p => p.ProjectIcon) + .Include(p => p.CallToAction); + queryableProjects = ApplyFilters(queryableProjects, skip, take, orderBy, orderByAsc, highlighted); - queryable = ApplyFilters(queryable, skip, take, orderBy, orderByAsc, highlighted); - List projects = await queryable.ToListAsync(); - return RedactUser(projects); + foreach(Project project in queryableProjects) + { + project.Collaborators = await GetDbSet() + .Where(p => p.ProjectId == project.Id) + .ToListAsync(); + project.User = RedactUser(await GetDbSet() + .Where(p => p.Id == project.UserId) + .FirstOrDefaultAsync()); + } + return await queryableProjects.ToListAsync(); } /// - /// Count the amount of projects matching the filters + /// Count the amount of projects matching the filters /// /// The highlighted filter /// The amount of projects matching the filters public virtual async Task CountAsync(bool? highlighted = null) { - return await ApplyFilters(DbSet, null, null, null, true, highlighted).CountAsync(); + return await ApplyFilters(DbSet, null, null, null, true, highlighted) + .CountAsync(); } /// @@ -222,44 +152,53 @@ public virtual async Task> SearchAsync( bool? highlighted = null ) { - List result = await ApplyFilters(GetProjectQueryable(query), skip, take, orderBy, orderByAsc, highlighted).ToListAsync(); - return result.Where(p => ProjectContainsQuery(p, query)).ToList(); + List result = + await ApplyFilters(GetProjectQueryable(query), skip, take, orderBy, orderByAsc, highlighted) + .ToListAsync(); + return result.Where(p => ProjectContainsQuery(p, query)) + .ToList(); } /// - /// Count the amount of projects matching the filters and the search query + /// Count the amount of projects matching the filters and the search query /// /// The search query /// The highlighted filter /// The amount of projects matching the filters public virtual async Task SearchCountAsync(string query, bool? highlighted = null) { - return await ApplyFilters(GetProjectQueryable(query), null, null, null, true, highlighted).CountAsync(); + return await ApplyFilters(GetProjectQueryable(query), null, null, null, true, highlighted) + .CountAsync(); } /// - /// Retrieve project with user and collaborators async. - /// Project will be redacted if user has that setting configured. + /// Retrieve project with user and collaborators async. + /// Project will be redacted if user has that setting configured. /// /// The identifier. /// - /// Possibly redacted Project object with user and collaborators + /// Possibly redacted Project object with user and collaborators /// public async Task FindWithUserAndCollaboratorsAsync(int id) { Project project = await GetDbSet() - .Include(p => p.User) - .Include(p => p.Collaborators) - .Include(p => p.ProjectIcon) - .Include(p => p.CallToAction) - .Where(p => p.Id == id) - .FirstOrDefaultAsync(); + .Include(p => p.User) + .Include(p => p.ProjectIcon) + .Include(p => p.CallToAction) + .Where(p => p.Id == id) + .FirstOrDefaultAsync(); + if(project != null) + { + project.Collaborators = await GetDbSet() + .Where(p => p.ProjectId == project.Id) + .ToListAsync(); + } return RedactUser(project); } /// - /// Updates the specified entity excluding the user object. + /// Updates the specified entity excluding the user object. /// /// The entity. public override void Update(Project entity) @@ -287,44 +226,151 @@ public override void Update(Project entity) } /// - /// Checks if any of the searchable fields of the project passed contains the provided query. + /// Redact user email from the Project if isPublic setting is set to false + /// + /// The project. + /// + /// Project with possibly redacted email depending on setting + /// + private Project RedactUser(Project project) + { + if(project == null) return null; + + if(project?.User?.IsPublic == false) + { + project.User.Email = Defaults.Privacy.RedactedEmail; + } + return project; + } + + /// + /// Redact user email from the User if isPublic setting is set to false + /// + /// The user. + /// + /// User with possibly redacted email depending on setting + /// + private User RedactUser(User user) + { + if(user.IsPublic == false) + { + user.Email = Defaults.Privacy.RedactedEmail; + } + return user; + } + + /// + /// Redact user email from the Projects in the list. + /// Email will only be redacted if isPublic setting is set to false. + /// + /// The projects. + /// + /// List of Projects with possibly redacted email depending on setting + /// + private List RedactUser(List projects) + { + for(int i = 0; i < projects.Count; i++) + { + projects[i] = RedactUser(projects[i]); + } + return projects; + } + + /// + /// Apply query parameters and find project based on these filters + /// + /// The linq queryable object. + /// The amount of objects to skip. + /// The amount of objects to take. + /// The order by expression. + /// if set to true [order by asc]. + /// Boolean if the project should show highlighted. + /// + /// IQueryable Projects based on the given filters + /// + private IQueryable ApplyFilters( + IQueryable queryable, + int? skip, + int? take, + Expression> orderBy, + bool orderByAsc, + bool? highlighted + ) + { + if(highlighted.HasValue) + { + IEnumerable highlightedQueryable = DbContext.Set() + .Where(h => h.StartDate <= DateTime.Now || + h.StartDate == null) + .Where(h => h.EndDate >= DateTime.Now || + h.EndDate == null) + .Select(h => h.ProjectId) + .ToList(); + + queryable = queryable.Where(p => highlightedQueryable.Contains(p.Id) == highlighted.Value); + } + + if(orderBy != null) + { + if(orderByAsc) + { + queryable = queryable.OrderBy(orderBy); + } else + { + queryable = queryable.OrderByDescending(orderBy); + } + } + + if(skip.HasValue) queryable = queryable.Skip(skip.Value); + if(take.HasValue) queryable = queryable.Take(take.Value); + return queryable; + } + + /// + /// Checks if any of the searchable fields of the project passed contains the provided query. /// /// A Project to search in /// The query to search in the project's searchable fields. - /// A boolean representing whether or not the passed query was found in the searchable fields of the provided project. + /// + /// A boolean representing whether or not the passed query was found in the searchable fields of the provided + /// project. + /// private static bool ProjectContainsQuery(Project project, string query) { Regex regex = new Regex(@$"\b{query}\b", RegexOptions.Singleline | RegexOptions.IgnoreCase); - return new List() - { - project.Name, - project.Description, - project.ShortDescription, - project.Uri, - project.User.Name, - project.Id.ToString() - } - .Any(text => regex.IsMatch(text)); + return new List + { + project.Name, + project.Description, + project.ShortDescription, + project.Uri, + project.User.Name, + project.Id.ToString() + } + .Any(text => regex.IsMatch(text)); } /// - /// Get the project queryable which contains the provided query. + /// Get the project queryable which contains the provided query. /// /// A string to search in the project's fields. /// The filtered IQueryable including the project user. private IQueryable GetProjectQueryable(string query) { return DbSet - .Include(p => p.User) - .Include(i => i.ProjectIcon) - .Include(p => p.CallToAction) - .Where(p => - p.Name.Contains(query) || - p.Description.Contains(query) || - p.ShortDescription.Contains(query) || - p.Uri.Contains(query) || - p.Id.ToString().Equals(query) || - p.User.Name.Contains(query)); + .Include(p => p.User) + .Include(i => i.ProjectIcon) + .Include(p => p.CallToAction) + .Where(p => + p.Name.Contains(query) || + p.Description.Contains(query) || + p.ShortDescription.Contains(query) || + p.Uri.Contains(query) || + p.Id.ToString() + .Equals(query) || + p.User.Name.Contains(query)); } + } + } From 96dbfad0579e3ea881294a881dfb3c9cf3fb76d5 Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Fri, 4 Dec 2020 21:51:15 +0100 Subject: [PATCH 2/5] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5356213..bb013a9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed issue where unused project icons where left in the database & File System - [#271](https://github.com/DigitalExcellence/dex-backend/issues/271) - Refactored Postman CLI files to make them work from Postman folder. - [#304](https://github.com/DigitalExcellence/dex-backend/issues/304) - Fixed issue where searching for a project did not include the project icon. - [#307](https://github.com/DigitalExcellence/dex-backend/issues/307) +- Fixes issue where retrieving projects performed badly due to large amount of collaborators. - [#331](https://github.com/DigitalExcellence/dex-backend/issues/331) ### Security From 4a87f55425358d90c55f776262d2406e4758a4bb Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Sat, 5 Dec 2020 12:38:46 +0100 Subject: [PATCH 3/5] Redact User should return null if no user is given --- Repositories/ProjectRepository.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Repositories/ProjectRepository.cs b/Repositories/ProjectRepository.cs index 96acb34b..b023e123 100644 --- a/Repositories/ProjectRepository.cs +++ b/Repositories/ProjectRepository.cs @@ -252,6 +252,7 @@ private Project RedactUser(Project project) /// private User RedactUser(User user) { + if(user == null) return null; if(user.IsPublic == false) { user.Email = Defaults.Privacy.RedactedEmail; From 50e1809bfd61e218bbb4d365ef031f05d7807600 Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Sat, 5 Dec 2020 12:59:16 +0100 Subject: [PATCH 4/5] Happy tests, happy life --- Repositories/ProjectRepository.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Repositories/ProjectRepository.cs b/Repositories/ProjectRepository.cs index b023e123..e5372cfe 100644 --- a/Repositories/ProjectRepository.cs +++ b/Repositories/ProjectRepository.cs @@ -105,6 +105,7 @@ public virtual async Task> GetAllWithUsersAndCollaboratorsAsync( ) { IQueryable queryableProjects = GetDbSet() + .Include(u => u.User) .Include(p => p.ProjectIcon) .Include(p => p.CallToAction); queryableProjects = ApplyFilters(queryableProjects, skip, take, orderBy, orderByAsc, highlighted); @@ -115,9 +116,6 @@ public virtual async Task> GetAllWithUsersAndCollaboratorsAsync( project.Collaborators = await GetDbSet() .Where(p => p.ProjectId == project.Id) .ToListAsync(); - project.User = RedactUser(await GetDbSet() - .Where(p => p.Id == project.UserId) - .FirstOrDefaultAsync()); } return await queryableProjects.ToListAsync(); } From c68bb826ede2779b9b1219722b1c8b29e227894e Mon Sep 17 00:00:00 2001 From: Brend Smits Date: Sat, 5 Dec 2020 13:04:53 +0100 Subject: [PATCH 5/5] Happy life, happy wife. Fixed issue where I did not redact user --- Repositories/ProjectRepository.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/Repositories/ProjectRepository.cs b/Repositories/ProjectRepository.cs index e5372cfe..53f3a674 100644 --- a/Repositories/ProjectRepository.cs +++ b/Repositories/ProjectRepository.cs @@ -116,6 +116,7 @@ public virtual async Task> GetAllWithUsersAndCollaboratorsAsync( project.Collaborators = await GetDbSet() .Where(p => p.ProjectId == project.Id) .ToListAsync(); + project.User = RedactUser(project.User); } return await queryableProjects.ToListAsync(); }