Reputation: 771
I am currently working on a web app using ASP.NET MVC and Angular. What I want to be able to do is get a list of navigation items for each user based on what their role is, store them all in a list of objects and serialize it into JSON to return to angular.
My method currently works just fine, but it takes at least 5 seconds for the data to hit the page while my page loads in less than a second. So my user is just kind of sitting there waiting for the navitems to popup which isn't good.
Here are the two methods that I use:
public NavDirectoryViewModel GetAllUserNavItems(string UserId)
{
NavDirectoryViewModel model = new NavDirectoryViewModel();
model.NavItems = new List<NavViewModel>();
foreach (var nav in GetAllNavItems())
{
if (GetUserNavItem(UserId, nav) != null)
{
if (nav.ParentId == 0)
model.NavItems.Add(GetUserNavItem(UserId, nav));
}
}
model.NavItems = model.NavItems.OrderBy(x => x.SortOrder).ToList();
return model;
}
public NavViewModel GetUserNavItem(string UserId, NavModel model)
{
try
{
if(model.AllowedUsers.FirstOrDefault(x => x.UserId.Equals(UserId)) != null || model.AllowedRoles.FirstOrDefault(x => x.Role.Users.FirstOrDefault(y => y.UserId.Equals(UserId)) != null) != null)
{
return new NavViewModel { Name = model.Name, Href = model.Href, Image = model.Image, SortOrder = model.SortOrder, SubItems = GetNavItems(model.Id).Where(x => x.ParentId == model.Id).Select(x => GetUserNavItem(UserId, x)).ToList() };
}
else
{
return null;
}
}
catch(Exception e1) { return null; }
}
Here are the two models:
public class NavViewModel
{
public string Name { get; set; }
public string Href { get; set; }
public string Image { get; set; }
public int? SortOrder { get; set; }
public List<NavViewModel> SubItems { get; set; }
}
public class NavModel
{
[Key]
public int Id { get; set; }
public string Name { get; set; }
public string Href { get; set; }
public string Image { get; set; }//string of server image path or fa class
public int ParentId { get; set; }//Will be 0 if it has no parent
public int? SortOrder { get; set; }
public virtual List<NavModel> SubItems { get; set; }
public virtual List<RoleNavAuthorizationModel> AllowedRoles { get; set; }
public virtual List<UserNavAuthorizationModel> AllowedUsers { get; set; }
}
The Role and UserNavAuthorizationModels only store the NavItem Id and the Role/UserId that is allowed to see the nav item.
Originally I was serializing the NavModel which was taking 10-15 seconds so I made a view model that didn't store as much data and that sped things up a little. However I can't have my users waiting for the navbar to load when everything else is already loaded.
When debugging I found that it is the GetUserNavItem
method that is the one that is taking a while. Everything else was running very quickly.
Does anyone have any suggestions on how to speed things up?
Update
Here is the code for GetAllNavItems()
:
public List<NavModel> GetAllNavItems() { return db.NavItems.ToList(); }
Update
I took Ondrej Svejdar's advice on caching and now store the JSON in the user's session so that if the page reloads while their session is active it takes no time at all to reload. I put this in my controller:
if (HttpContext.Cache[User.Identity.GetUserId() + "NAV"] == null)
{
string NavJSON = JsonConvert.SerializeObject(nrepo.GetAllUserNavItems(User.Identity.GetUserId()), Formatting.Indented, new JsonSerializerSettings() { ReferenceLoopHandling = ReferenceLoopHandling.Ignore });
HttpContext.Cache[User.Identity.GetUserId() + "NAV"] = NavJSON;
return Json(NavJSON, JsonRequestBehavior.AllowGet);
}
else
{
string NavJSON = HttpContext.Cache[User.Identity.GetUserId() + "NAV"].ToString();
return Json(NavJSON, JsonRequestBehavior.AllowGet);
}
Upvotes: 2
Views: 188
Reputation: 7783
Well, you should rethink your model. You are mixing Security concerns with actual [navigation data] which is why you are forced to bring down so much information just to authenticate the user.
That being said, you have a lot of chained LINQ queries which probably result in a lot of SQL being executed if you look under the covers. Try this instead:
public NavDirectoryViewModel GetAllUserNavItems(string userId)
{
NavDirectoryViewModel model = new NavDirectoryViewModel();
model.Items = GetNavItems(0, userId).OrderBy(x => x.SortOrder);
return model;
}
private static IEnumerable<NavViewModel> GetNavItems(int parentId, string userId)
{
List<NavViewModel> items = new List<NavViewModel>();
var children = GetAllNavItems(parentId);
foreach (var child in children)
{
if (child.IsUserAllowed(userId))
{
var navItem = new NavViewModel() { Name = child.Name, Href = child.Href, Image = child.Image, SortOrder = child.SortOrder };
navItem.SubItems = GetNavItems(child.Id, userId);
items.Add(navItem);
}
}
return items;
}
You will need to modify your GetAllNavItems so that it only pulls the navigation items you need at a particular stage. For instance, top level menus (ParentId = 0). You can then just use a recursive method to populate everything.
You will also need to add a method to your NavModel class:
public bool IsUserAllowed(string userId)
{
return AllowedUsers.Any(x => x.UserId.Equals(userId)) || AllowedRoles.SelectMany(r => r.Role.Users).Any(y => y.UserId.Equals(userId));
}
Notice the use of Any vs FirstOrDefault in the LINQ queries.
This modifications should give you decent speed because you won't be pulling data you don't need and you will be avoiding all the nested queries. You were also calling ToList() in quite a few places which causes the query to execute and the data to be projected into objects.
Still, your model needs reworking IMHO. Way too much data being pulled.
I whipped this out in a few minutes so check for errors/semantics but this should give you a good idea of how to improve performance.
Upvotes: 1