Merge pull request #18823 from abpframework/liangshiwei/settings

Improve `ValueProvider` system.

framework/src/Volo.Abp.Authorization/Volo/Abp/Authorization/Permissions/PermissionValueProviderManager.cs

@@ -13,19 +13,31 @@ public class PermissionValueProviderManager : IPermissionValueProviderManager, I
     private readonly Lazy<List<IPermissionValueProvider>> _lazyProviders;
 
     protected AbpPermissionOptions Options { get; }
+    protected IServiceProvider ServiceProvider { get; }
 
     public PermissionValueProviderManager(
         IServiceProvider serviceProvider,
         IOptions<AbpPermissionOptions> options)
     {
         Options = options.Value;
+        ServiceProvider = serviceProvider;
 
-        _lazyProviders = new Lazy<List<IPermissionValueProvider>>(
-            () => Options
-                .ValueProviders
-                .Select(c => serviceProvider.GetRequiredService(c) as IPermissionValueProvider)
-                .ToList()!,
-            true
-        );
+        _lazyProviders = new Lazy<List<IPermissionValueProvider>>(GetProviders, true);
+    }
+
+    protected virtual List<IPermissionValueProvider> GetProviders()
+    {
+        var providers = Options
+            .ValueProviders
+            .Select(type => (ServiceProvider.GetRequiredService(type) as IPermissionValueProvider)!)
+            .ToList();
+
+        var multipleProviders = providers.GroupBy(p => p.Name).FirstOrDefault(x => x.Count() > 1);
+        if(multipleProviders != null)
+        {
+            throw new AbpException($"Duplicate permission value provider name detected: {multipleProviders.Key}. Providers:{Environment.NewLine}{multipleProviders.Select(p => p.GetType().FullName!).JoinAsString(Environment.NewLine)}");
+        }
+
+        return providers;
     }
 }

framework/src/Volo.Abp.Features/Volo/Abp/Features/FeatureChecker.cs

@@ -12,34 +12,26 @@ public class FeatureChecker : FeatureCheckerBase
     protected AbpFeatureOptions Options { get; }
     protected IServiceProvider ServiceProvider { get; }
     protected IFeatureDefinitionManager FeatureDefinitionManager { get; }
-    protected List<IFeatureValueProvider> Providers => _providers.Value;
-
-    private readonly Lazy<List<IFeatureValueProvider>> _providers;
+    protected IFeatureValueProviderManager FeatureValueProviderManager { get; }
 
     public FeatureChecker(
         IOptions<AbpFeatureOptions> options,
         IServiceProvider serviceProvider,
-        IFeatureDefinitionManager featureDefinitionManager)
+        IFeatureDefinitionManager featureDefinitionManager,
+        IFeatureValueProviderManager featureValueProviderManager)
     {
         ServiceProvider = serviceProvider;
         FeatureDefinitionManager = featureDefinitionManager;
+        FeatureValueProviderManager = featureValueProviderManager;
 
         Options = options.Value;
-
-        _providers = new Lazy<List<IFeatureValueProvider>>(
-            () => Options
-                .ValueProviders
-                .Select(type => (ServiceProvider.GetRequiredService(type) as IFeatureValueProvider)!)
-                .ToList(),
-            true
-        );
     }
 
     public override async Task<string?> GetOrNullAsync(string name)
     {
         var featureDefinition = await FeatureDefinitionManager.GetAsync(name);
-        var providers = Enumerable
-            .Reverse(Providers);
+        var providers = FeatureValueProviderManager.ValueProviders
+            .Reverse();
 
         if (featureDefinition.AllowedProviders.Any())
         {

framework/src/Volo.Abp.Features/Volo/Abp/Features/FeatureValueProviderManager.cs

@@ -0,0 +1,43 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
+using Volo.Abp.DependencyInjection;
+
+namespace Volo.Abp.Features;
+
+public class FeatureValueProviderManager : IFeatureValueProviderManager, ISingletonDependency
+{
+    public IReadOnlyList<IFeatureValueProvider> ValueProviders => _lazyProviders.Value;
+    private readonly Lazy<List<IFeatureValueProvider>> _lazyProviders;
+
+    protected AbpFeatureOptions Options { get; }
+    protected IServiceProvider ServiceProvider { get; }
+
+    public FeatureValueProviderManager(
+        IServiceProvider serviceProvider,
+        IOptions<AbpFeatureOptions> options)
+    {
+        Options = options.Value;
+        ServiceProvider = serviceProvider;
+
+        _lazyProviders = new Lazy<List<IFeatureValueProvider>>(GetProviders, true);
+    }
+
+    protected virtual List<IFeatureValueProvider> GetProviders()
+    {
+        var providers = Options
+            .ValueProviders
+            .Select(type => (ServiceProvider.GetRequiredService(type) as IFeatureValueProvider)!)
+            .ToList();
+
+        var multipleProviders = providers.GroupBy(p => p.Name).FirstOrDefault(x => x.Count() > 1);
+        if(multipleProviders != null)
+        {
+            throw new AbpException($"Duplicate feature value provider name detected: {multipleProviders.Key}. Providers:{Environment.NewLine}{multipleProviders.Select(p => p.GetType().FullName!).JoinAsString(Environment.NewLine)}");
+        }
+
+        return providers;
+    }
+}

framework/src/Volo.Abp.Features/Volo/Abp/Features/IFeatureValueProviderManager.cs

@@ -0,0 +1,8 @@
+using System.Collections.Generic;
+
+namespace Volo.Abp.Features;
+
+public interface IFeatureValueProviderManager
+{
+    IReadOnlyList<IFeatureValueProvider> ValueProviders { get; }
+}

framework/src/Volo.Abp.Settings/Volo/Abp/Settings/SettingValueProviderManager.cs

@@ -10,7 +10,9 @@ namespace Volo.Abp.Settings;
 public class SettingValueProviderManager : ISettingValueProviderManager, ISingletonDependency
 {
     public List<ISettingValueProvider> Providers => _lazyProviders.Value;
+
     protected AbpSettingOptions Options { get; }
+    protected IServiceProvider ServiceProvider { get; }
     private readonly Lazy<List<ISettingValueProvider>> _lazyProviders;
 
     public SettingValueProviderManager(
@@ -19,13 +21,24 @@ public SettingValueProviderManager(
     {
 
         Options = options.Value;
+        ServiceProvider = serviceProvider;
+
+        _lazyProviders = new Lazy<List<ISettingValueProvider>>(GetProviders, true);
+    }
+
+    protected virtual List<ISettingValueProvider> GetProviders()
+    {
+        var providers = Options
+            .ValueProviders
+            .Select(type => (ServiceProvider.GetRequiredService(type) as ISettingValueProvider)!)
+            .ToList();
+
+        var multipleProviders = providers.GroupBy(p => p.Name).FirstOrDefault(x => x.Count() > 1);
+        if(multipleProviders != null)
+        {
+            throw new AbpException($"Duplicate setting value provider name detected: {multipleProviders.Key}. Providers:{Environment.NewLine}{multipleProviders.Select(p => p.GetType().FullName!).JoinAsString(Environment.NewLine)}");
+        }
 
-        _lazyProviders = new Lazy<List<ISettingValueProvider>>(
-            () => Options
-                .ValueProviders
-                .Select(type => serviceProvider.GetRequiredService(type) as ISettingValueProvider)
-                .ToList()!,
-            true
-        );
+        return providers;
     }
 }

framework/test/Volo.Abp.Authorization.Tests/Volo/Abp/Authorization/PermissionValueProviderManager_Tests.cs

@@ -0,0 +1,59 @@
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Volo.Abp.Authorization;
+using Volo.Abp.Authorization.Permissions;
+using Volo.Abp.Authorization.TestServices;
+using Xunit;
+
+namespace Volo.Abp;
+
+public class PermissionValueProviderManager_Tests: AuthorizationTestBase
+{
+    private readonly IPermissionValueProviderManager _permissionValueProviderManager;
+
+    public PermissionValueProviderManager_Tests()
+    {
+        _permissionValueProviderManager = GetRequiredService<IPermissionValueProviderManager>();
+    }
+
+    protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
+    {
+        options.Services.Configure<AbpPermissionOptions>(permissionOptions =>
+        {
+            permissionOptions.ValueProviders.Add<TestDuplicatePermissionValueProvider>();
+        });
+    }
+
+    [Fact]
+    public void Should_Throw_Exception_If_Duplicate_Provider_Name_Detected()
+    {
+        var exception = Assert.Throws<AbpException>(() =>
+        {
+            var providers = _permissionValueProviderManager.ValueProviders;
+        });
+
+        exception.Message.ShouldBe($"Duplicate permission value provider name detected: TestPermissionValueProvider1. Providers:{Environment.NewLine}{typeof(TestDuplicatePermissionValueProvider).FullName}{Environment.NewLine}{typeof(TestPermissionValueProvider1).FullName}");
+    }
+}
+
+public class TestDuplicatePermissionValueProvider : PermissionValueProvider
+{
+    public TestDuplicatePermissionValueProvider(IPermissionStore permissionStore) : base(permissionStore)
+    {
+    }
+
+    public override string Name => "TestPermissionValueProvider1";
+
+    public override Task<PermissionGrantResult> CheckAsync(PermissionValueCheckContext context)
+    {
+        throw new NotImplementedException();
+    }
+
+    public override Task<MultiplePermissionGrantResult> CheckAsync(PermissionValuesCheckContext context)
+    {
+        throw new NotImplementedException();
+    }
+}

framework/test/Volo.Abp.Features.Tests/Volo/Abp/Features/FeatureValueProviderManager_Tests.cs

@@ -0,0 +1,54 @@
+using System;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Xunit;
+
+namespace Volo.Abp.Features;
+
+public class FeatureValueProviderManager_Tests : FeatureTestBase
+{
+    private readonly IFeatureValueProviderManager _featureValueProviderManager;
+
+    public FeatureValueProviderManager_Tests()
+    {
+        _featureValueProviderManager = GetRequiredService<IFeatureValueProviderManager>();
+    }
+
+    protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
+    {
+        options.Services.Configure<AbpFeatureOptions>(permissionOptions =>
+        {
+            permissionOptions.ValueProviders.Add<TestDuplicateFeatureValueProvider>();
+        });
+    }
+
+    [Fact]
+    public void Should_Throw_Exception_If_Duplicate_Provider_Name_Detected()
+    {
+        var exception = Assert.Throws<AbpException>(() =>
+        {
+            var providers = _featureValueProviderManager.ValueProviders;
+        });
+
+        exception.Message.ShouldBe($"Duplicate feature value provider name detected: {TestDuplicateFeatureValueProvider.ProviderName}. Providers:{Environment.NewLine}{typeof(TestDuplicateFeatureValueProvider).FullName}{Environment.NewLine}{typeof(DefaultValueFeatureValueProvider).FullName}");
+    }
+}
+
+public class TestDuplicateFeatureValueProvider : FeatureValueProvider
+{
+    public const string ProviderName = "D";
+
+    public override string Name => ProviderName;
+
+    public TestDuplicateFeatureValueProvider(IFeatureStore settingStore)
+        : base(settingStore)
+    {
+
+    }
+
+    public override Task<string> GetOrNullAsync(FeatureDefinition setting)
+    {
+        throw new NotImplementedException();
+    }
+}

framework/test/Volo.Abp.Settings.Tests/Volo/Abp/Settings/SettingValueProviderManager_Tests.cs

@@ -0,0 +1,62 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Microsoft.Extensions.DependencyInjection;
+using Shouldly;
+using Volo.Abp.DependencyInjection;
+using Volo.Abp.Testing;
+using Xunit;
+
+namespace Volo.Abp.Settings;
+
+public class SettingValueProviderManager_Tests: AbpIntegratedTest<AbpSettingsTestModule>
+{
+    private readonly ISettingValueProviderManager _settingValueProviderManager;
+
+    public SettingValueProviderManager_Tests()
+    {
+        _settingValueProviderManager = GetRequiredService<ISettingValueProviderManager>();
+    }
+
+    protected override void SetAbpApplicationCreationOptions(AbpApplicationCreationOptions options)
+    {
+        options.UseAutofac();
+        options.Services.Configure<AbpSettingOptions>(settingOptions =>
+        {
+            settingOptions.ValueProviders.Add<TestDuplicateSettingValueProvider>();
+        });
+    }
+
+    [Fact]
+    public void Should_Throw_Exception_If_Duplicate_Provider_Name_Detected()
+    {
+        var exception = Assert.Throws<AbpException>(() =>
+        {
+            var providers = _settingValueProviderManager.Providers;
+        });
+
+        exception.Message.ShouldBe($"Duplicate setting value provider name detected: {TestDuplicateSettingValueProvider.ProviderName}. Providers:{Environment.NewLine}{typeof(TestDuplicateSettingValueProvider).FullName}{Environment.NewLine}{typeof(TestSettingValueProvider).FullName}");
+    }
+}
+
+public class TestDuplicateSettingValueProvider : ISettingValueProvider, ITransientDependency
+{
+    public const string ProviderName = "Test";
+
+
+    public string Name => ProviderName;
+
+    public TestDuplicateSettingValueProvider()
+    {
+    }
+
+    public Task<string> GetOrNullAsync(SettingDefinition setting)
+    {
+        throw new NotImplementedException();
+    }
+
+    public Task<List<SettingValue>> GetAllAsync(SettingDefinition[] settings)
+    {
+        throw new NotImplementedException();
+    }
+}

modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManager.cs

@@ -144,7 +144,7 @@ public virtual async Task SetAsync(
 
         if (!providers.Any())
         {
-            return;
+            throw new AbpException($"Unknown feature value provider: {providerName}");
         }
 
         if (providers.Count > 1 && !forceToSet && value != null)

modules/feature-management/test/Volo.Abp.FeatureManagement.Domain.Tests/Volo/Abp/FeatureManagement/FeatureManager_Tests.cs

@@ -209,4 +209,15 @@ public async Task DeleteAsync()
 
         (await _featureManager.GetOrNullAsync(TestFeatureDefinitionProvider.BackupCount, TenantFeatureValueProvider.ProviderName, TestEditionIds.TenantId.ToString())).ShouldBe("0");
     }
+
+    [Fact]
+    public async Task Set_Should_Throw_Exception_If_Provider_Not_Found()
+    {
+        var exception = await Assert.ThrowsAsync<AbpException>(async () =>
+        {
+            await _featureManager.SetAsync(TestFeatureDefinitionProvider.EmailSupport, "true", "UndefinedProvider", "Test");
+        });
+
+        exception.Message.ShouldBe("Unknown feature value provider: UndefinedProvider");
+    }
 }