From 0399035996fe99836e6be98abd6deb9f53bf5146 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Halil=20=C4=B0brahim=20Kalkan?= Date: Thu, 15 Apr 2021 16:11:56 +0300 Subject: [PATCH] Refactored permission state provider system --- .../Permissions/PermissionDefinition.cs | 3 ++ .../PermissionDefinitionExtensions.cs | 29 +++++------------ .../Permissions/PermissionStateManager.cs | 12 +++---- .../CachedServiceProvider.cs | 31 +++++++++++++++++++ .../ICachedServiceProvider.cs | 15 +++++++++ .../PermissionDefinitionExtensions.cs | 20 ++++++++++-- .../RequireFeaturesPermissionStateProvider.cs | 21 ++++++++----- .../GlobalFeatureDefinitionExtensions.cs | 21 +++++++++++-- ...reGlobalFeaturesPermissionStateProvider.cs | 24 +++++++++----- .../PermissionStateProvider_Tests.cs | 2 +- ...izationTestPermissionDefinitionProvider.cs | 2 +- 11 files changed, 128 insertions(+), 52 deletions(-) create mode 100644 framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/CachedServiceProvider.cs create mode 100644 framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/ICachedServiceProvider.cs diff --git a/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs b/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs index 38873fe1b8..ac525367ad 100644 --- a/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs +++ b/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinition.cs @@ -31,6 +31,8 @@ namespace Volo.Abp.Authorization.Permissions /// public List Providers { get; } //TODO: Rename to AllowedProviders? + public List StateProviders { get; } + public ILocalizableString DisplayName { get => _displayName; @@ -86,6 +88,7 @@ namespace Volo.Abp.Authorization.Permissions Properties = new Dictionary(); Providers = new List(); + StateProviders = new List(); _children = new List(); } diff --git a/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinitionExtensions.cs b/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinitionExtensions.cs index 34e0c4b68b..7e5de5a1f1 100644 --- a/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinitionExtensions.cs +++ b/framework/src/Volo.Abp.Authorization.Abstractions/Volo/Abp/Authorization/Permissions/PermissionDefinitionExtensions.cs @@ -1,34 +1,19 @@ -using System.Collections.Generic; -using JetBrains.Annotations; +using JetBrains.Annotations; namespace Volo.Abp.Authorization.Permissions { public static class PermissionDefinitionExtensions { - public const string PropertyName = "_AbpPermissionStateProviders"; - - public static PermissionDefinition AddStateProvider( + public static PermissionDefinition AddStateProviders( [NotNull] this PermissionDefinition permissionDefinition, [NotNull] params IPermissionStateProvider[] permissionStateProviders) { - var stateProviders = permissionDefinition.GetStateProvidersInternal(); - - foreach (var provider in permissionStateProviders) - { - stateProviders.AddIfNotContains(provider); - } - + Check.NotNull(permissionDefinition, nameof(permissionDefinition)); + Check.NotNull(permissionStateProviders, nameof(permissionStateProviders)); + + permissionDefinition.StateProviders.AddRange(permissionStateProviders); + return permissionDefinition; } - - public static IReadOnlyList GetStateProviders([NotNull] this PermissionDefinition permissionDefinition) - { - return permissionDefinition.GetStateProvidersInternal(); - } - - private static List GetStateProvidersInternal([NotNull] this PermissionDefinition permissionDefinition) - { - return (List) permissionDefinition.Properties.GetOrAdd(PropertyName, () => new List()); - } } } diff --git a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionStateManager.cs b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionStateManager.cs index 0729e57de3..957ed3c7a6 100644 --- a/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionStateManager.cs +++ b/framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionStateManager.cs @@ -25,18 +25,14 @@ namespace Volo.Abp.Authorization.Permissions var context = new PermissionStateContext { Permission = permission, - ServiceProvider = scope.ServiceProvider + ServiceProvider = scope.ServiceProvider.GetRequiredService() }; - var providers = permission.GetStateProviders(); - if (providers != null && providers.Any()) + foreach (var provider in permission.StateProviders) { - foreach (var provider in providers) + if (!await provider.IsEnabledAsync(context)) { - if (!await provider.IsEnabledAsync(context)) - { - return false; - } + return false; } } diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/CachedServiceProvider.cs b/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/CachedServiceProvider.cs new file mode 100644 index 0000000000..5e24d547e0 --- /dev/null +++ b/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/CachedServiceProvider.cs @@ -0,0 +1,31 @@ +using System; +using System.Collections.Generic; + +namespace Volo.Abp.DependencyInjection +{ + [ExposeServices(typeof(ICachedServiceProvider))] + public class CachedServiceProvider : ICachedServiceProvider, IScopedDependency + { + protected IServiceProvider ServiceProvider { get; } + + protected IDictionary CachedServices { get; } + + public CachedServiceProvider(IServiceProvider serviceProvider) + { + ServiceProvider = serviceProvider; + + CachedServices = new Dictionary + { + {typeof(IServiceProvider), serviceProvider} + }; + } + + public object GetService(Type serviceType) + { + return CachedServices.GetOrAdd( + serviceType, + () => ServiceProvider.GetService(serviceType) + ); + } + } +} \ No newline at end of file diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/ICachedServiceProvider.cs b/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/ICachedServiceProvider.cs new file mode 100644 index 0000000000..be8a3df18d --- /dev/null +++ b/framework/src/Volo.Abp.Core/Volo/Abp/DependencyInjection/ICachedServiceProvider.cs @@ -0,0 +1,15 @@ +using System; + +namespace Volo.Abp.DependencyInjection +{ + /// + /// Provides services by caching the resolved services. + /// It caches all type of services including transients. + /// This service's lifetime is scoped and it should be used + /// for a limited scope. + /// + public interface ICachedServiceProvider : IServiceProvider + { + + } +} \ No newline at end of file diff --git a/framework/src/Volo.Abp.Features/Volo/Abp/Features/PermissionDefinitionExtensions.cs b/framework/src/Volo.Abp.Features/Volo/Abp/Features/PermissionDefinitionExtensions.cs index a070126d91..a213232f2f 100644 --- a/framework/src/Volo.Abp.Features/Volo/Abp/Features/PermissionDefinitionExtensions.cs +++ b/framework/src/Volo.Abp.Features/Volo/Abp/Features/PermissionDefinitionExtensions.cs @@ -1,14 +1,28 @@ -using Volo.Abp.Authorization.Permissions; +using JetBrains.Annotations; +using Volo.Abp.Authorization.Permissions; namespace Volo.Abp.Features { public static class FeatureDefinitionExtensions { - public static PermissionDefinition RequireFeatures(this PermissionDefinition permissionDefinition, params string[] features) + public static PermissionDefinition RequireFeatures( + [NotNull] this PermissionDefinition permissionDefinition, + params string[] features) { + return permissionDefinition.RequireFeatures(true, features); + } + + public static PermissionDefinition RequireFeatures( + [NotNull] this PermissionDefinition permissionDefinition, + bool requiresAll, + params string[] features) + { + Check.NotNull(permissionDefinition, nameof(permissionDefinition)); Check.NotNullOrEmpty(features, nameof(features)); - return permissionDefinition.AddStateProvider(new RequireFeaturesPermissionStateProvider(features)); + return permissionDefinition.AddStateProviders( + new RequireFeaturesPermissionStateProvider(requiresAll, features) + ); } } } diff --git a/framework/src/Volo.Abp.Features/Volo/Abp/Features/RequireFeaturesPermissionStateProvider.cs b/framework/src/Volo.Abp.Features/Volo/Abp/Features/RequireFeaturesPermissionStateProvider.cs index 93c8287cae..0fee54c598 100644 --- a/framework/src/Volo.Abp.Features/Volo/Abp/Features/RequireFeaturesPermissionStateProvider.cs +++ b/framework/src/Volo.Abp.Features/Volo/Abp/Features/RequireFeaturesPermissionStateProvider.cs @@ -1,5 +1,4 @@ -using System.Collections.Generic; -using System.Threading.Tasks; +using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Volo.Abp.Authorization.Permissions; @@ -7,19 +6,27 @@ namespace Volo.Abp.Features { public class RequireFeaturesPermissionStateProvider : IPermissionStateProvider { - private readonly List _requireFeatures = new List(); + private readonly string[] _featureNames; + private readonly bool _requiresAll; - public RequireFeaturesPermissionStateProvider(params string[] requireFeatures) + public RequireFeaturesPermissionStateProvider(params string[] featureNames) + : this(true, featureNames) { - Check.NotNullOrEmpty(requireFeatures, nameof(requireFeatures)); + + } + + public RequireFeaturesPermissionStateProvider(bool requiresAll, params string[] featureNames) + { + Check.NotNullOrEmpty(featureNames, nameof(featureNames)); - _requireFeatures.AddRange(requireFeatures); + _requiresAll = requiresAll; + _featureNames = featureNames; } public async Task IsEnabledAsync(PermissionStateContext context) { var feature = context.ServiceProvider.GetRequiredService(); - return await feature.IsEnabledAsync(true, _requireFeatures.ToArray()); + return await feature.IsEnabledAsync(_requiresAll, _featureNames); } } } diff --git a/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/GlobalFeatureDefinitionExtensions.cs b/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/GlobalFeatureDefinitionExtensions.cs index ada52fad0b..200f002e07 100644 --- a/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/GlobalFeatureDefinitionExtensions.cs +++ b/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/GlobalFeatureDefinitionExtensions.cs @@ -1,14 +1,29 @@ -using Volo.Abp.Authorization.Permissions; +using JetBrains.Annotations; +using Volo.Abp.Authorization.Permissions; namespace Volo.Abp.GlobalFeatures { public static class GlobalFeatureDefinitionExtensions { - public static PermissionDefinition RequireGlobalFeatures(this PermissionDefinition permissionDefinition, params string[] globalFeatures) + public static PermissionDefinition RequireGlobalFeatures( + this PermissionDefinition permissionDefinition, + params string[] globalFeatures) { + return permissionDefinition.RequireGlobalFeatures(true, globalFeatures); + } + + public static PermissionDefinition RequireGlobalFeatures( + [NotNull] this PermissionDefinition permissionDefinition, + bool requiresAll, + params string[] globalFeatures) + { + Check.NotNull(permissionDefinition, nameof(permissionDefinition)); Check.NotNullOrEmpty(globalFeatures, nameof(globalFeatures)); - return permissionDefinition.AddStateProvider(new RequireGlobalFeaturesPermissionStateProvider(globalFeatures)); + return permissionDefinition.AddStateProviders( + new RequireGlobalFeaturesPermissionStateProvider(requiresAll, globalFeatures) + ); } + } } diff --git a/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/RequireGlobalFeaturesPermissionStateProvider.cs b/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/RequireGlobalFeaturesPermissionStateProvider.cs index b7aea021c9..40df2a14b3 100644 --- a/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/RequireGlobalFeaturesPermissionStateProvider.cs +++ b/framework/src/Volo.Abp.GlobalFeatures/Volo/Abp/GlobalFeatures/RequireGlobalFeaturesPermissionStateProvider.cs @@ -1,5 +1,4 @@ -using System.Collections.Generic; -using System.Linq; +using System.Linq; using System.Threading.Tasks; using Volo.Abp.Authorization.Permissions; @@ -7,18 +6,29 @@ namespace Volo.Abp.GlobalFeatures { public class RequireGlobalFeaturesPermissionStateProvider : IPermissionStateProvider { - private readonly List _requireGlobalFeatures = new List(); + private readonly string[] _globalFeatureNames; + private readonly bool _requiresAll; - public RequireGlobalFeaturesPermissionStateProvider(params string[] requireGlobalFeatures) + public RequireGlobalFeaturesPermissionStateProvider(params string[] globalFeatureNames) + : this(true, globalFeatureNames) { - Check.NotNullOrEmpty(requireGlobalFeatures, nameof(requireGlobalFeatures)); + } + + public RequireGlobalFeaturesPermissionStateProvider(bool requiresAll, params string[] globalFeatureNames) + { + Check.NotNullOrEmpty(globalFeatureNames, nameof(globalFeatureNames)); - _requireGlobalFeatures.AddRange(requireGlobalFeatures); + _requiresAll = requiresAll; + _globalFeatureNames = globalFeatureNames; } public Task IsEnabledAsync(PermissionStateContext context) { - return Task.FromResult(_requireGlobalFeatures.All(x => GlobalFeatureManager.Instance.IsEnabled(x))); + bool isEnabled = _requiresAll + ? _globalFeatureNames.All(x => GlobalFeatureManager.Instance.IsEnabled(x)) + : _globalFeatureNames.Any(x => GlobalFeatureManager.Instance.IsEnabled(x)); + + return Task.FromResult(isEnabled); } } } diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionStateProvider_Tests.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionStateProvider_Tests.cs index 1ab609cd87..f7ab858ac1 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionStateProvider_Tests.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionStateProvider_Tests.cs @@ -29,7 +29,7 @@ namespace Volo.Abp.Authorization public async Task PermissionState_Test() { var myPermission1 = PermissionDefinitionManager.Get("MyPermission1"); - myPermission1.GetStateProviders().ShouldContain(x => x.GetType() == typeof(TestRequireEditionPermissionStateProvider)); + myPermission1.StateProviders.ShouldContain(x => x.GetType() == typeof(TestRequireEditionPermissionStateProvider)); (await PermissionStateManager.IsEnabledAsync(myPermission1)).ShouldBeFalse(); diff --git a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs index 8198119a6d..d67cf8febc 100644 --- a/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs +++ b/framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/TestServices/AuthorizationTestPermissionDefinitionProvider.cs @@ -17,7 +17,7 @@ namespace Volo.Abp.Authorization.TestServices group.AddPermission("MyAuthorizedService1"); - group.AddPermission("MyPermission1").AddStateProvider(new TestRequireEditionPermissionStateProvider()); + group.AddPermission("MyPermission1").AddStateProviders(new TestRequireEditionPermissionStateProvider()); group.AddPermission("MyPermission2"); group.GetPermissionOrNull("MyAuthorizedService1").ShouldNotBeNull();