From 27bcb83b04d5bdb682ef329647d0d97afd4eaa4e Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 24 Feb 2025 11:54:27 +0800 Subject: [PATCH 1/3] Optimize the database queries for permission checks. --- .../RolePermissionManagementProvider.cs | 54 ++++++++------- .../PermissionAppService.cs | 68 ++++++++----------- .../PermissionManagementProvider.cs | 26 ++++--- 3 files changed, 73 insertions(+), 75 deletions(-) diff --git a/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs b/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs index b51110204a..e97d524cd7 100644 --- a/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs +++ b/modules/identity/src/Volo.Abp.PermissionManagement.Domain.Identity/Volo/Abp/PermissionManagement/Identity/RolePermissionManagementProvider.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Volo.Abp.Authorization.Permissions; +using Volo.Abp.Domain.Repositories; using Volo.Abp.Guids; using Volo.Abp.Identity; using Volo.Abp.MultiTenancy; @@ -37,41 +38,44 @@ public class RolePermissionManagementProvider : PermissionManagementProvider public async override Task CheckAsync(string[] names, string providerName, string providerKey) { - var multiplePermissionValueProviderGrantInfo = new MultiplePermissionValueProviderGrantInfo(names); - var permissionGrants = new List(); - - if (providerName == Name) + using (PermissionGrantRepository.DisableTracking()) { - permissionGrants.AddRange(await PermissionGrantRepository.GetListAsync(names, providerName, providerKey)); + var multiplePermissionValueProviderGrantInfo = new MultiplePermissionValueProviderGrantInfo(names); + var permissionGrants = new List(); - } + if (providerName == Name) + { + permissionGrants.AddRange(await PermissionGrantRepository.GetListAsync(names, providerName, providerKey)); - if (providerName == UserPermissionValueProvider.ProviderName) - { - var userId = Guid.Parse(providerKey); - var roleNames = await UserRoleFinder.GetRoleNamesAsync(userId); + } - foreach (var roleName in roleNames) + if (providerName == UserPermissionValueProvider.ProviderName) { - permissionGrants.AddRange(await PermissionGrantRepository.GetListAsync(names, Name, roleName)); + var userId = Guid.Parse(providerKey); + var roleNames = await UserRoleFinder.GetRoleNamesAsync(userId); + + foreach (var roleName in roleNames) + { + permissionGrants.AddRange(await PermissionGrantRepository.GetListAsync(names, Name, roleName)); + } } - } - permissionGrants = permissionGrants.Distinct().ToList(); - if (!permissionGrants.Any()) - { - return multiplePermissionValueProviderGrantInfo; - } + permissionGrants = permissionGrants.Distinct().ToList(); + if (!permissionGrants.Any()) + { + return multiplePermissionValueProviderGrantInfo; + } - foreach (var permissionName in names) - { - var permissionGrant = permissionGrants.FirstOrDefault(x => x.Name == permissionName); - if (permissionGrant != null) + foreach (var permissionName in names) { - multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(true, permissionGrant.ProviderKey); + var permissionGrant = permissionGrants.FirstOrDefault(x => x.Name == permissionName); + if (permissionGrant != null) + { + multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(true, permissionGrant.ProviderKey); + } } - } - return multiplePermissionValueProviderGrantInfo; + return multiplePermissionValueProviderGrantInfo; + } } } diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs index ef056918c8..0b7b61c326 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs @@ -43,72 +43,62 @@ public class PermissionAppService : ApplicationService, IPermissionAppService }; var multiTenancySide = CurrentTenant.GetMultiTenancySide(); + var permissionGroups = new List(); foreach (var group in await PermissionDefinitionManager.GetGroupsAsync()) { var groupDto = CreatePermissionGroupDto(group); + var permissions = group.GetPermissionsWithChildren() + .Where(x => x.IsEnabled && + (!x.Providers.Any() || x.Providers.Contains(providerName)) && + x.MultiTenancySide.HasFlag(multiTenancySide)); var neededCheckPermissions = new List(); - - var permissions = group.GetPermissionsWithChildren() - .Where(x => x.IsEnabled) - .Where(x => !x.Providers.Any() || x.Providers.Contains(providerName)) - .Where(x => x.MultiTenancySide.HasFlag(multiTenancySide)); - foreach (var permission in permissions) { - if(permission.Parent != null && !neededCheckPermissions.Contains(permission.Parent)) + if (permission.Parent != null && !neededCheckPermissions.Contains(permission.Parent)) { continue; } - + if (await SimpleStateCheckerManager.IsEnabledAsync(permission)) { neededCheckPermissions.Add(permission); } } - if (!neededCheckPermissions.Any()) - { - continue; - } - - var grantInfoDtos = neededCheckPermissions - .Select(CreatePermissionGrantInfoDto) - .ToList(); + groupDto.Permissions.AddRange(neededCheckPermissions.Select(CreatePermissionGrantInfoDto)); + permissionGroups.Add(groupDto); + } - var multipleGrantInfo = await PermissionManager.GetAsync(neededCheckPermissions.Select(x => x.Name).ToArray(), providerName, providerKey); + var multipleGrantInfo = await PermissionManager.GetAsync( + permissionGroups.SelectMany(group => group.Permissions).Select(permission => permission.Name).ToArray(), + providerName, + providerKey); - foreach (var grantInfo in multipleGrantInfo.Result) + foreach (var permissionGroup in permissionGroups) + { + foreach (var permission in permissionGroup.Permissions.Where(x => multipleGrantInfo.Result.Any(y => y.Name == x.Name))) { - var grantInfoDto = grantInfoDtos.First(x => x.Name == grantInfo.Name); - - grantInfoDto.IsGranted = grantInfo.IsGranted; - - foreach (var provider in grantInfo.Providers) + var grantInfo = multipleGrantInfo.Result.First(x => x.Name == permission.Name); + permission.IsGranted = grantInfo.IsGranted; + permission.GrantedProviders = grantInfo.Providers.Select(x => new ProviderInfoDto { - grantInfoDto.GrantedProviders.Add(new ProviderInfoDto - { - ProviderName = provider.Name, - ProviderKey = provider.Key, - }); - } - - groupDto.Permissions.Add(grantInfoDto); + ProviderName = x.Name, + ProviderKey = x.Key, + }).ToList(); } - if (groupDto.Permissions.Any()) - { - result.Groups.Add(groupDto); - } + result.Groups.Add(permissionGroup); } return result; } - private PermissionGrantInfoDto CreatePermissionGrantInfoDto(PermissionDefinition permission) + protected virtual PermissionGrantInfoDto CreatePermissionGrantInfoDto(PermissionDefinition permission) { - return new PermissionGrantInfoDto { + return new PermissionGrantInfoDto + { Name = permission.Name, DisplayName = permission.DisplayName?.Localize(StringLocalizerFactory), ParentName = permission.Parent?.Name, @@ -117,10 +107,10 @@ public class PermissionAppService : ApplicationService, IPermissionAppService }; } - private PermissionGroupDto CreatePermissionGroupDto(PermissionGroupDefinition group) + protected virtual PermissionGroupDto CreatePermissionGroupDto(PermissionGroupDefinition group) { var localizableDisplayName = group.DisplayName as LocalizableString; - + return new PermissionGroupDto { Name = group.Name, diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs index 0a80ed53e5..b6dfc2e518 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManagementProvider.cs @@ -1,5 +1,6 @@ using System.Linq; using System.Threading.Tasks; +using Volo.Abp.Domain.Repositories; using Volo.Abp.Guids; using Volo.Abp.MultiTenancy; @@ -34,21 +35,24 @@ public abstract class PermissionManagementProvider : IPermissionManagementProvid public virtual async Task CheckAsync(string[] names, string providerName, string providerKey) { - var multiplePermissionValueProviderGrantInfo = new MultiplePermissionValueProviderGrantInfo(names); - if (providerName != Name) + using (PermissionGrantRepository.DisableTracking()) { - return multiplePermissionValueProviderGrantInfo; - } + var multiplePermissionValueProviderGrantInfo = new MultiplePermissionValueProviderGrantInfo(names); + if (providerName != Name) + { + return multiplePermissionValueProviderGrantInfo; + } - var permissionGrants = await PermissionGrantRepository.GetListAsync(names, providerName, providerKey); + var permissionGrants = await PermissionGrantRepository.GetListAsync(names, providerName, providerKey); - foreach (var permissionName in names) - { - var isGrant = permissionGrants.Any(x => x.Name == permissionName); - multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(isGrant, providerKey); - } + foreach (var permissionName in names) + { + var isGrant = permissionGrants.Any(x => x.Name == permissionName); + multiplePermissionValueProviderGrantInfo.Result[permissionName] = new PermissionValueProviderGrantInfo(isGrant, providerKey); + } - return multiplePermissionValueProviderGrantInfo; + return multiplePermissionValueProviderGrantInfo; + } } public virtual Task SetAsync(string name, string providerKey, bool isGranted) From 2b0e68f6e1891325455018b9adbcc97849171868 Mon Sep 17 00:00:00 2001 From: maliming Date: Mon, 24 Feb 2025 14:01:37 +0800 Subject: [PATCH 2/3] Refactor permission filtering logic for improved readability and performance. --- .../PermissionAppService.cs | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs index 0b7b61c326..aa9bfe0029 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Application/Volo/Abp/PermissionManagement/PermissionAppService.cs @@ -49,9 +49,9 @@ public class PermissionAppService : ApplicationService, IPermissionAppService { var groupDto = CreatePermissionGroupDto(group); var permissions = group.GetPermissionsWithChildren() - .Where(x => x.IsEnabled && - (!x.Providers.Any() || x.Providers.Contains(providerName)) && - x.MultiTenancySide.HasFlag(multiTenancySide)); + .Where(x => x.IsEnabled) + .Where(x => !x.Providers.Any() || x.Providers.Contains(providerName)) + .Where(x => x.MultiTenancySide.HasFlag(multiTenancySide)); var neededCheckPermissions = new List(); foreach (var permission in permissions) @@ -67,6 +67,11 @@ public class PermissionAppService : ApplicationService, IPermissionAppService } } + if (!neededCheckPermissions.Any()) + { + continue; + } + groupDto.Permissions.AddRange(neededCheckPermissions.Select(CreatePermissionGrantInfoDto)); permissionGroups.Add(groupDto); } @@ -78,9 +83,14 @@ public class PermissionAppService : ApplicationService, IPermissionAppService foreach (var permissionGroup in permissionGroups) { - foreach (var permission in permissionGroup.Permissions.Where(x => multipleGrantInfo.Result.Any(y => y.Name == x.Name))) + foreach (var permission in permissionGroup.Permissions) { - var grantInfo = multipleGrantInfo.Result.First(x => x.Name == permission.Name); + var grantInfo = multipleGrantInfo.Result.FirstOrDefault(x => x.Name == permission.Name); + if (grantInfo == null) + { + continue; + } + permission.IsGranted = grantInfo.IsGranted; permission.GrantedProviders = grantInfo.Providers.Select(x => new ProviderInfoDto { @@ -89,7 +99,10 @@ public class PermissionAppService : ApplicationService, IPermissionAppService }).ToList(); } - result.Groups.Add(permissionGroup); + if (permissionGroup.Permissions.Any()) + { + result.Groups.Add(permissionGroup); + } } return result; From 41fa6ce9c7a11d3473a76b6d2219b2421f1472c2 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 25 Feb 2025 15:36:19 +0800 Subject: [PATCH 3/3] Always disabling tracking for `PermissionGrantRepository` queries. --- .../PermissionDataSeeder.cs | 16 ++-- .../PermissionManagement/PermissionStore.cs | 85 ++++++++++--------- 2 files changed, 56 insertions(+), 45 deletions(-) diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeeder.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeeder.cs index 1e8a4aab7b..de2b824412 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeeder.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionDataSeeder.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; using Volo.Abp.DependencyInjection; +using Volo.Abp.Domain.Repositories; using Volo.Abp.Guids; using Volo.Abp.MultiTenancy; @@ -33,14 +34,17 @@ public class PermissionDataSeeder : IPermissionDataSeeder, ITransientDependency { using (CurrentTenant.Change(tenantId)) { - var names = grantedPermissions.ToArray(); - var existsPermissionGrants = (await PermissionGrantRepository.GetListAsync(names, providerName, providerKey)).Select(x => x.Name).ToList(); - var permissions = names.Except(existsPermissionGrants).Select(permissionName => new PermissionGrant(GuidGenerator.Create(), permissionName, providerName, providerKey, tenantId)).ToList(); - if (!permissions.Any()) + using (PermissionGrantRepository.DisableTracking()) { - return; + var names = grantedPermissions.ToArray(); + var existsPermissionGrants = (await PermissionGrantRepository.GetListAsync(names, providerName, providerKey)).Select(x => x.Name).ToList(); + var permissions = names.Except(existsPermissionGrants).Select(permissionName => new PermissionGrant(GuidGenerator.Create(), permissionName, providerName, providerKey, tenantId)).ToList(); + if (!permissions.Any()) + { + return; + } + await PermissionGrantRepository.InsertManyAsync(permissions); } - await PermissionGrantRepository.InsertManyAsync(permissions); } } } diff --git a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs index 6eed394a31..85ffd77924 100644 --- a/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs +++ b/modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionStore.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Volo.Abp.Authorization.Permissions; using Volo.Abp.Caching; using Volo.Abp.DependencyInjection; +using Volo.Abp.Domain.Repositories; namespace Volo.Abp.PermissionManagement; @@ -67,36 +68,39 @@ public class PermissionStore : IPermissionStore, ITransientDependency string currentName, PermissionGrantCacheItem currentCacheItem) { - var permissions = await PermissionDefinitionManager.GetPermissionsAsync(); + using (PermissionGrantRepository.DisableTracking()) + { + var permissions = await PermissionDefinitionManager.GetPermissionsAsync(); - Logger.LogDebug($"Getting all granted permissions from the repository for this provider name,key: {providerName},{providerKey}"); + Logger.LogDebug($"Getting all granted permissions from the repository for this provider name,key: {providerName},{providerKey}"); - var grantedPermissionsHashSet = new HashSet( - (await PermissionGrantRepository.GetListAsync(providerName, providerKey)).Select(p => p.Name) - ); + var grantedPermissionsHashSet = new HashSet( + (await PermissionGrantRepository.GetListAsync(providerName, providerKey)).Select(p => p.Name) + ); - Logger.LogDebug($"Setting the cache items. Count: {permissions.Count}"); + Logger.LogDebug($"Setting the cache items. Count: {permissions.Count}"); - var cacheItems = new List>(); + var cacheItems = new List>(); - foreach (var permission in permissions) - { - var isGranted = grantedPermissionsHashSet.Contains(permission.Name); + foreach (var permission in permissions) + { + var isGranted = grantedPermissionsHashSet.Contains(permission.Name); - cacheItems.Add(new KeyValuePair( - CalculateCacheKey(permission.Name, providerName, providerKey), - new PermissionGrantCacheItem(isGranted)) - ); + cacheItems.Add(new KeyValuePair( + CalculateCacheKey(permission.Name, providerName, providerKey), + new PermissionGrantCacheItem(isGranted)) + ); - if (permission.Name == currentName) - { - currentCacheItem.IsGranted = isGranted; + if (permission.Name == currentName) + { + currentCacheItem.IsGranted = isGranted; + } } - } - await Cache.SetManyAsync(cacheItems); + await Cache.SetManyAsync(cacheItems); - Logger.LogDebug($"Finished setting the cache items. Count: {permissions.Count}"); + Logger.LogDebug($"Finished setting the cache items. Count: {permissions.Count}"); + } } public virtual async Task IsGrantedAsync(string[] names, string providerName, string providerKey) @@ -169,34 +173,37 @@ public class PermissionStore : IPermissionStore, ITransientDependency string providerKey, List notCacheKeys) { - var permissions = (await PermissionDefinitionManager.GetPermissionsAsync()) - .Where(x => notCacheKeys.Any(k => GetPermissionNameFormCacheKeyOrNull(k) == x.Name)).ToList(); + using (PermissionGrantRepository.DisableTracking()) + { + var permissions = (await PermissionDefinitionManager.GetPermissionsAsync()) + .Where(x => notCacheKeys.Any(k => GetPermissionNameFormCacheKeyOrNull(k) == x.Name)).ToList(); - Logger.LogDebug($"Getting not cache granted permissions from the repository for this provider name,key: {providerName},{providerKey}"); + Logger.LogDebug($"Getting not cache granted permissions from the repository for this provider name,key: {providerName},{providerKey}"); - var grantedPermissionsHashSet = new HashSet( - (await PermissionGrantRepository.GetListAsync(notCacheKeys.Select(GetPermissionNameFormCacheKeyOrNull).ToArray(), providerName, providerKey)).Select(p => p.Name) - ); + var grantedPermissionsHashSet = new HashSet( + (await PermissionGrantRepository.GetListAsync(notCacheKeys.Select(GetPermissionNameFormCacheKeyOrNull).ToArray(), providerName, providerKey)).Select(p => p.Name) + ); - Logger.LogDebug($"Setting the cache items. Count: {permissions.Count}"); + Logger.LogDebug($"Setting the cache items. Count: {permissions.Count}"); - var cacheItems = new List>(); + var cacheItems = new List>(); - foreach (var permission in permissions) - { - var isGranted = grantedPermissionsHashSet.Contains(permission.Name); + foreach (var permission in permissions) + { + var isGranted = grantedPermissionsHashSet.Contains(permission.Name); - cacheItems.Add(new KeyValuePair( - CalculateCacheKey(permission.Name, providerName, providerKey), - new PermissionGrantCacheItem(isGranted)) - ); - } + cacheItems.Add(new KeyValuePair( + CalculateCacheKey(permission.Name, providerName, providerKey), + new PermissionGrantCacheItem(isGranted)) + ); + } - await Cache.SetManyAsync(cacheItems); + await Cache.SetManyAsync(cacheItems); - Logger.LogDebug($"Finished setting the cache items. Count: {permissions.Count}"); + Logger.LogDebug($"Finished setting the cache items. Count: {permissions.Count}"); - return cacheItems; + return cacheItems; + } } protected virtual string CalculateCacheKey(string name, string providerName, string providerKey)