From d986cc6509f91d6eaf0371610df9eeeb8fbdf5f3 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 10 Nov 2020 14:02:26 +0800 Subject: [PATCH] Refactor. --- .../Permissions/PermissionChecker.cs | 38 +++++++++---------- .../RolePermissionValueProvider.cs | 14 +++---- .../Volo/Abp/Settings/SettingProvider.cs | 3 +- .../Abp/SettingManagement/SettingCacheItem.cs | 4 +- .../SettingManagementStore.cs | 2 +- .../SettingCacheItem_Tests.cs | 16 ++++++++ 6 files changed, 44 insertions(+), 33 deletions(-) create mode 100644 modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingCacheItem_Tests.cs diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs index 3f61df0b7e..bfdcf5fd66 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionChecker.cs @@ -100,36 +100,34 @@ namespace Volo.Abp.Authorization.Permissions foreach (var name in names) { var permission = PermissionDefinitionManager.Get(name); - if (!permission.IsEnabled || !permission.MultiTenancySide.HasFlag(multiTenancySide)) - { - result.Result.Add(name, PermissionGrantResult.Undefined); - continue; - } result.Result.Add(name, PermissionGrantResult.Undefined); - permissionDefinitions.Add(permission); + + if (permission.IsEnabled && permission.MultiTenancySide.HasFlag(multiTenancySide)) + { + permissionDefinitions.Add(permission); + } } foreach (var provider in PermissionValueProviderManager.ValueProviders) { - var context = new PermissionValuesCheckContext(permissionDefinitions.Where(x => !x.Providers.Any() || x.Providers.Contains(provider.Name)).ToList(), + var context = new PermissionValuesCheckContext( + permissionDefinitions.Where(x => !x.Providers.Any() || x.Providers.Contains(provider.Name)).ToList(), claimsPrincipal); var multipleResult = await provider.CheckAsync(context); - foreach (var grantResult in multipleResult.Result) + foreach (var grantResult in multipleResult.Result.Where(grantResult => + result.Result.ContainsKey(grantResult.Key) && + result.Result[grantResult.Key] == PermissionGrantResult.Undefined && + grantResult.Value != PermissionGrantResult.Undefined)) + { + result.Result[grantResult.Key] = grantResult.Value; + permissionDefinitions.RemoveAll(x => x.Name == grantResult.Key); + } + + if (result.AllGranted || result.AllProhibited) { - if (result.Result.ContainsKey(grantResult.Key) && - result.Result[grantResult.Key] == PermissionGrantResult.Undefined && - grantResult.Value != PermissionGrantResult.Undefined) - { - result.Result[grantResult.Key] = grantResult.Value; - permissionDefinitions.RemoveAll(x => x.Name == grantResult.Key); - } - - if (result.AllGranted || result.AllProhibited) - { - break; - } + break; } } diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/RolePermissionValueProvider.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/RolePermissionValueProvider.cs index e8187f762a..e3ab7629a2 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/RolePermissionValueProvider.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/RolePermissionValueProvider.cs @@ -51,15 +51,13 @@ namespace Volo.Abp.Authorization.Permissions foreach (var role in roles) { var multipleResult = await PermissionStore.IsGrantedAsync(permissionNames.ToArray(), Name, role); - foreach (var grantResult in multipleResult.Result) + foreach (var grantResult in multipleResult.Result.Where(grantResult => + result.Result.ContainsKey(grantResult.Key) && + result.Result[grantResult.Key] == PermissionGrantResult.Undefined && + grantResult.Value != PermissionGrantResult.Undefined)) { - if (result.Result.ContainsKey(grantResult.Key) && - result.Result[grantResult.Key] == PermissionGrantResult.Undefined && - grantResult.Value != PermissionGrantResult.Undefined) - { - result.Result[grantResult.Key] = grantResult.Value; - permissionNames.RemoveAll(x => x == grantResult.Key); - } + result.Result[grantResult.Key] = grantResult.Value; + permissionNames.RemoveAll(x => x == grantResult.Key); } if (result.AllGranted || result.AllProhibited) diff --git a/framework/src/Volo.Abp.Settings/Volo/Abp/Settings/SettingProvider.cs b/framework/src/Volo.Abp.Settings/Volo/Abp/Settings/SettingProvider.cs index f7cf9fbb1c..d9ebb4e757 100644 --- a/framework/src/Volo.Abp.Settings/Volo/Abp/Settings/SettingProvider.cs +++ b/framework/src/Volo.Abp.Settings/Volo/Abp/Settings/SettingProvider.cs @@ -55,8 +55,7 @@ namespace Volo.Abp.Settings var notNullValues = settingValues.Where(x => x.Value != null).ToList(); foreach (var settingValue in notNullValues) { - var value = settingValue; - var settingDefinition = settingDefinitions.First(x => x.Name == value.Name); + var settingDefinition = settingDefinitions.First(x => x.Name == settingValue.Name); if (settingDefinition.IsEncrypted) { settingValue.Value = SettingEncryptionService.Decrypt(settingDefinition, settingValue.Value); diff --git a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingCacheItem.cs b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingCacheItem.cs index 88b1eebbb9..3b6b968e19 100644 --- a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingCacheItem.cs +++ b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingCacheItem.cs @@ -28,10 +28,10 @@ namespace Volo.Abp.SettingManagement return string.Format(CacheKeyFormat, providerName, providerKey, name); } - public static string GetSettingNameFormCacheKey(string cacheKey) + public static string GetSettingNameFormCacheKeyOrNull(string cacheKey) { var result = FormattedStringValueExtracter.Extract(cacheKey, CacheKeyFormat, true); - return result.IsMatch ? result.Matches.Last().Value : cacheKey; + return result.IsMatch ? result.Matches.Last().Value : null; } } } diff --git a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManagementStore.cs b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManagementStore.cs index 0dd33f0ffd..baad579594 100644 --- a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManagementStore.cs +++ b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManagementStore.cs @@ -207,7 +207,7 @@ namespace Volo.Abp.SettingManagement protected virtual string GetSettingNameFormCacheKeyOrNull(string key) { //TODO: throw ex when name is null? - return SettingCacheItem.GetSettingNameFormCacheKey(key); + return SettingCacheItem.GetSettingNameFormCacheKeyOrNull(key); } } } diff --git a/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingCacheItem_Tests.cs b/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingCacheItem_Tests.cs new file mode 100644 index 0000000000..390851e08e --- /dev/null +++ b/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingCacheItem_Tests.cs @@ -0,0 +1,16 @@ +using Shouldly; +using Xunit; + +namespace Volo.Abp.SettingManagement +{ + public class SettingCacheItem_Tests + { + [Fact] + public void GetSettingNameFormCacheKeyOrNull() + { + var key = SettingCacheItem.CalculateCacheKey("aaa", "bbb", "ccc"); + SettingCacheItem.GetSettingNameFormCacheKeyOrNull(key).ShouldBe("aaa"); + SettingCacheItem.GetSettingNameFormCacheKeyOrNull("aaabbbccc").ShouldBeNull(); + } + } +}