Further optimise CategoryService

src/Libraries/Nop.Services/Catalog/Caching/CategoryCacheEventConsumer.cs

@@ -17,10 +17,6 @@ public partial class CategoryCacheEventConsumer : CacheEventConsumer<Category>
  /// <returns>A task that represents the asynchronous operation</returns>
  protected override async Task ClearCacheAsync(Category entity, EntityEventType entityEventType)
  {
- await RemoveByPrefixAsync(NopCatalogDefaults.CategoriesByParentCategoryPrefix, entity);
- await RemoveByPrefixAsync(NopCatalogDefaults.CategoriesByParentCategoryPrefix, entity.ParentCategoryId);
- await RemoveByPrefixAsync(NopCatalogDefaults.CategoriesChildIdsPrefix, entity);
- await RemoveByPrefixAsync(NopCatalogDefaults.CategoriesChildIdsPrefix, entity.ParentCategoryId);
  await RemoveByPrefixAsync(NopCatalogDefaults.CategoriesHomepagePrefix);
  await RemoveByPrefixAsync(NopCatalogDefaults.CategoryBreadcrumbPrefix);
  await RemoveByPrefixAsync(NopCatalogDefaults.CategoryProductsNumberPrefix);

src/Libraries/Nop.Services/Catalog/CategoryService.cs

@@ -156,6 +156,43 @@ public partial class CategoryService : ICategoryService
  yield return orphan;
  }
 
+ /// <summary>
+ /// Gets all descendant categories of a category
+ /// </summary>
+ /// <param name="categoriesByParentId">A dictionary of categories, accessible by parent ID</param>
+ /// <param name="categoryId">Category identifier</param>
+ /// <returns>All descendant categories as an IEnumerable</returns>
+ protected static IEnumerable<Category> GetChildCategories(IDictionary<int, IList<Category>> categoriesByParentId, int categoryId)
+ {
+ if (categoriesByParentId.TryGetValue(categoryId, out var categories))
+ {
+ foreach (var category in categories)
+ {
+ yield return category;
+ foreach (var child in GetChildCategories(categoriesByParentId, category.Id))
+ yield return child;
+ }
+ }
+ }
+
+ /// <summary>
+ /// Gets a lookup of child categories for a specified store
+ /// </summary>
+ /// <param name="storeId">Store identifier</param>
+ /// <param name="showHidden">A value indicating whether to show hidden records</param>
+ /// <returns>A lookup of child categories accessible by parent ID</returns>
+ protected async Task<IDictionary<int, IList<Category>>> GetChildCategoryLookupAsync(int storeId, bool showHidden = false)
+ {
+ return await _staticCacheManager.GetAsync(
+ _staticCacheManager.PrepareKeyForDefaultCache(NopCatalogDefaults.ChildCategoryLookupCacheKey, storeId, showHidden),
+ async () => (await GetAllCategoriesAsync(storeId: storeId, showHidden: showHidden))
+ .ToGroupedDictionary(c => c.ParentCategoryId, (x, y) => 
+ {
+ var cmp = x.DisplayOrder.CompareTo(y.DisplayOrder);
+ return cmp == 0 ? x.Id.CompareTo(y.Id) : cmp;
+ }));
+ }
+
  #endregion
 
  #region Methods
@@ -294,29 +331,8 @@ public virtual async Task<IList<Category>> GetAllCategoriesAsync(int storeId = 0
  bool showHidden = false)
  {
  var store = await _storeContext.GetCurrentStoreAsync();
- var customer = await _workContext.GetCurrentCustomerAsync();
- var customerRoleIds = await _customerService.GetCustomerRoleIdsAsync(customer);
-
- var categories = await _categoryRepository.GetAllAsync(async query =>
- {
- if (!showHidden)
- {
- query = query.Where(c => c.Published);
-
- //apply store mapping constraints
- query = await _storeMappingService.ApplyStoreMapping(query, store.Id);
-
- //apply ACL constraints
- query = await _aclService.ApplyAcl(query, customerRoleIds);
- }
-
- query = query.Where(c => !c.Deleted && c.ParentCategoryId == parentCategoryId);
-
- return query.OrderBy(c => c.DisplayOrder).ThenBy(c => c.Id);
- }, cache => cache.PrepareKeyForDefaultCache(NopCatalogDefaults.CategoriesByParentCategoryCacheKey,
- parentCategoryId, showHidden, customerRoleIds, store));
-
- return categories;
+ var categoriesByParentId = await GetChildCategoryLookupAsync(store.Id, showHidden);
+ return categoriesByParentId.TryGetValue(parentCategoryId, out var categories) ? categories : new List<Category>();
  }
 
  /// <summary>
@@ -406,34 +422,9 @@ await GetChildCategoryIdsAsync(categoryId, store.Id))
  /// </returns>
  public virtual async Task<IList<int>> GetChildCategoryIdsAsync(int parentCategoryId, int storeId = 0, bool showHidden = false)
  {
- var cacheKey = _staticCacheManager.PrepareKeyForDefaultCache(NopCatalogDefaults.CategoriesChildIdsCacheKey,
- parentCategoryId,
- await _customerService.GetCustomerRoleIdsAsync(await _workContext.GetCurrentCustomerAsync()),
- storeId,
- showHidden);
-
- return await _staticCacheManager.GetAsync(cacheKey, async () =>
- {
- //little hack for performance optimization
- //there's no need to invoke "GetAllCategoriesByParentCategoryId" multiple times (extra SQL commands) to load childs
- //so we load all categories at once (we know they are cached) and process them server-side
- var lookup = await _staticCacheManager.GetAsync(
- _staticCacheManager.PrepareKeyForDefaultCache(NopCatalogDefaults.ChildCategoryIdLookupCacheKey, storeId, showHidden),
- async () => (await GetAllCategoriesAsync(storeId: storeId, showHidden: showHidden))
- .ToGroupedDictionary(c => c.ParentCategoryId, x => x.Id));
-
- var categoryIds = new List<int>();
- if (lookup.TryGetValue(parentCategoryId, out var categories))
- {
- categoryIds.AddRange(categories);
- var childCategoryIds = categories.SelectAwait(async cId => await GetChildCategoryIdsAsync(cId, storeId, showHidden));
- // avoid allocating a new list or blocking with ToEnumerable
- await foreach (var cIds in childCategoryIds)
- categoryIds.AddRange(cIds);
- }
-
- return categoryIds;
- });
+ return GetChildCategories(await GetChildCategoryLookupAsync(storeId, showHidden), parentCategoryId)
+ .Select(c => c.Id)
+ .ToList();
  }
 
  /// <summary>
@@ -779,25 +770,26 @@ public virtual async Task<IList<Category>> GetCategoryBreadCrumbAsync(Category c
 
  return await _staticCacheManager.GetAsync(breadcrumbCacheKey, async () =>
  {
+ var allCategoriesById = (allCategories ?? await GetAllCategoriesAsync(showHidden: showHidden))
+ .DistinctBy(c => c.Id)
+ .ToDictionary(c => c.Id);
  var result = new List<Category>();
 
  //used to prevent circular references
- var alreadyProcessedCategoryIds = new List<int>();
+ var alreadyProcessedCategoryIds = new HashSet<int>();
 
  while (category != null && //not null
  !category.Deleted && //not deleted
  (showHidden || category.Published) && //published
+ !alreadyProcessedCategoryIds.Contains(category.Id) && //prevent circular references
  (showHidden || await _aclService.AuthorizeAsync(category)) && //ACL
- (showHidden || await _storeMappingService.AuthorizeAsync(category)) && //Store mapping
- !alreadyProcessedCategoryIds.Contains(category.Id)) //prevent circular references
+ (showHidden || await _storeMappingService.AuthorizeAsync(category))) //Store mapping
  {
  result.Add(category);
 
  alreadyProcessedCategoryIds.Add(category.Id);
 
- category = allCategories != null
- ? allCategories.FirstOrDefault(c => c.Id == category.ParentCategoryId)
- : await GetCategoryByIdAsync(category.ParentCategoryId);
+ category = allCategoriesById.GetValueOrDefault(category.ParentCategoryId);
  }
 
  result.Reverse();

src/Libraries/Nop.Services/Catalog/NopCatalogDefaults.cs

@@ -43,62 +43,24 @@ public static partial class NopCatalogDefaults
 
  #region Categories
 
- /// <summary>
- /// Gets a key for caching
- /// </summary>
- /// <remarks>
- /// {0} : parent category ID
- /// {1} : show hidden records?
- /// {2} : roles of the current user
- /// {3} : store ID
- /// </remarks>
- public static CacheKey CategoriesByParentCategoryCacheKey => new("Nop.category.byparent.{0}-{1}-{2}-{3}", CategoriesByParentCategoryPrefix);
-
- /// <summary>
- /// Gets a key pattern to clear cache
- /// </summary>
- /// <remarks>
- /// {0} : parent category ID
- /// </remarks>
- public static string CategoriesByParentCategoryPrefix => "Nop.category.byparent.{0}";
-
- /// <summary>
- /// Gets a key for caching
- /// </summary>
- /// <remarks>
- /// {0} : parent category id
- /// {1} : roles of the current user
- /// {2} : current store ID
- /// {3} : show hidden records?
- /// </remarks>
- public static CacheKey CategoriesChildIdsCacheKey => new("Nop.category.childids.{0}-{1}-{2}-{3}", CategoriesChildIdsPrefix);
-
- /// <summary>
- /// Gets a key pattern to clear cache
- /// </summary>
- /// <remarks>
- /// {0} : parent category ID
- /// </remarks>
- public static string CategoriesChildIdsPrefix => "Nop.category.childids.{0}";
-
  /// <summary>
  /// Gets a key for caching
  /// </summary>
  /// <remarks>
  /// {0} : current store ID
  /// {1} : show hidden records?
  /// </remarks>
- public static CacheKey ChildCategoryIdLookupCacheKey => new("Nop.childcategoryidlookup.bystore.{0}-{1}", ChildCategoryIdLookupPrefix, ChildCategoryIdLookupByStorePrefix);
+ public static CacheKey ChildCategoryLookupCacheKey => new("Nop.childcategorylookup.bystore.{0}-{1}", ChildCategoryLookupPrefix, ChildCategoryLookupByStorePrefix);
 
  /// <summary>
  /// Gets a key pattern to clear cache
  /// </summary>
- public static string ChildCategoryIdLookupPrefix => "Nop.childcategoryidlookup.";
+ public static string ChildCategoryLookupPrefix => "Nop.childcategorylookup.";
 
  /// <summary>
  /// Gets a key pattern to clear cache
  /// </summary>
- public static string ChildCategoryIdLookupByStorePrefix => "Nop.childcategoryidlookup.bystore.{0}";
+ public static string ChildCategoryLookupByStorePrefix => "Nop.childcategorylookup.bystore.{0}";
 
  /// <summary>
  /// Gets a key for caching

src/Libraries/Nop.Services/Extensions.cs

@@ -67,11 +67,13 @@ public static class Extensions
  /// <param name="xs">List of objects</param>
  /// <param name="keySelector">A key-selector function</param>
  /// <param name="valueSelector">A value-selector function</param>
+ /// <param name="comparer">An optional comparison function for sorting the value lists</param>
  /// <returns>A dictionary with values grouped by key</returns>
  public static IDictionary<TKey, IList<TValue>> ToGroupedDictionary<T, TKey, TValue>(
  this IEnumerable<T> xs,
  Func<T, TKey> keySelector,
- Func<T, TValue> valueSelector)
+ Func<T, TValue> valueSelector,
+ Comparison<TValue> comparer = null)
  {
  var result = new Dictionary<TKey, IList<TValue>>();
 
@@ -86,6 +88,12 @@ public static class Extensions
  result[key] = new List<TValue> { value };
  }
 
+ if (comparer != null)
+ {
+ foreach (var list in result.Values)
+ ((List<TValue>)list).Sort(comparer); // sort in-place
+ }
+
  return result;
  }
 
@@ -99,9 +107,10 @@ public static class Extensions
  /// <returns>A dictionary with values grouped by key</returns>
  public static IDictionary<TKey, IList<T>> ToGroupedDictionary<T, TKey>(
  this IEnumerable<T> xs,
- Func<T, TKey> keySelector)
+ Func<T, TKey> keySelector,
+ Comparison<T> comparer = null)
  {
- return xs.ToGroupedDictionary(keySelector, x => x);
+ return xs.ToGroupedDictionary(keySelector, x => x, comparer);
  }
  }
 }

src/Libraries/Nop.Services/Stores/Caching/StoreMappingCacheEventConsumer.cs

@@ -22,7 +22,7 @@ protected override async Task ClearCacheAsync(StoreMapping entity)
  await RemoveAsync(NopStoreDefaults.StoreMappingExistsCacheKey, entity.EntityName);
 
  if (entity.EntityName.Equals(nameof(Category)))
- await RemoveByPrefixAsync(NopCatalogDefaults.ChildCategoryIdLookupByStorePrefix, entity.StoreId);
+ await RemoveByPrefixAsync(NopCatalogDefaults.ChildCategoryLookupByStorePrefix, entity.StoreId);
  }
  }
 }

src/Presentation/Nop.Web/Areas/Admin/Controllers/CategoryController.cs

@@ -327,11 +327,7 @@ public virtual async Task<IActionResult> Edit(CategoryModel model, bool continue
 
  //if parent category changes, we need to clear cache for previous parent category
  if (category.ParentCategoryId != model.ParentCategoryId)
- {
- await _staticCacheManager.RemoveByPrefixAsync(NopCatalogDefaults.CategoriesByParentCategoryPrefix, category.ParentCategoryId);
- await _staticCacheManager.RemoveByPrefixAsync(NopCatalogDefaults.CategoriesChildIdsPrefix, category.ParentCategoryId);
- await _staticCacheManager.RemoveByPrefixAsync(NopCatalogDefaults.ChildCategoryIdLookupPrefix);
- }
+ await _staticCacheManager.RemoveByPrefixAsync(NopCatalogDefaults.ChildCategoryLookupPrefix);
 
  category = model.ToEntity(category);
  category.UpdatedOnUtc = DateTime.UtcNow;
@@ -619,4 +615,4 @@ public virtual async Task<IActionResult> ProductAddPopup(AddProductToCategoryMod
 
  #endregion
  }
-}
+}