Merge pull request #25462 from abpframework/fix/feature-allowed-providers-respected-by-management

Respect AllowedProviders/Providers in Feature/Setting management

modules/feature-management/src/Volo.Abp.FeatureManagement.Application/Volo/Abp/FeatureManagement/FeatureAppService.cs

@@ -38,6 +38,7 @@ public class FeatureAppService : FeatureManagementAppServiceBase, IFeatureAppSer
         {
             var groupDto = CreateFeatureGroupDto(group);
 
+            var includedFeatures = new HashSet<FeatureDefinition>();
             foreach (var featureDefinition in group.GetFeaturesWithChildren())
             {
                 if (providerName == TenantFeatureValueProvider.ProviderName &&
@@ -48,6 +49,18 @@ public class FeatureAppService : FeatureManagementAppServiceBase, IFeatureAppSer
                     continue;
                 }
 
+                if (featureDefinition.AllowedProviders.Any() &&
+                    !featureDefinition.AllowedProviders.Contains(providerName))
+                {
+                    continue;
+                }
+
+                if (featureDefinition.Parent != null && !includedFeatures.Contains(featureDefinition.Parent))
+                {
+                    continue;
+                }
+
+                includedFeatures.Add(featureDefinition);
                 var feature = await FeatureManager.GetOrNullWithProviderAsync(featureDefinition.Name, providerName, providerKey);
                 groupDto.Features.Add(CreateFeatureDto(feature, featureDefinition));
             }

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

@@ -93,8 +93,17 @@ public class FeatureManager : IFeatureManager, ISingletonDependency
 
         foreach (var feature in featureDefinitions)
         {
+            var featureProviderList = feature.AllowedProviders.Any()
+                ? providerList.Where(p => feature.AllowedProviders.Contains(p.Name)).ToList()
+                : providerList;
+
+            if (!featureProviderList.Any())
+            {
+                continue;
+            }
+
             var featureNameValueWithGrantedProvider = new FeatureNameValueWithGrantedProvider(feature.Name, null);
-            foreach (var provider in providerList)
+            foreach (var provider in featureProviderList)
             {
                 string pk = null;
                 if (provider.Compatible(providerName))
@@ -137,6 +146,11 @@ public class FeatureManager : IFeatureManager, ISingletonDependency
             throw new FeatureValueInvalidException(feature.DisplayName.Localize(StringLocalizerFactory));
         }
 
+        if (feature.AllowedProviders.Any() && !feature.AllowedProviders.Contains(providerName))
+        {
+            throw new AbpException($"The feature named '{name}' is not compatible with the provider named '{providerName}'");
+        }
+
         var providers = Enumerable
             .Reverse(Providers)
             .SkipWhile(p => p.Name != providerName)
@@ -195,6 +209,11 @@ public class FeatureManager : IFeatureManager, ISingletonDependency
             providers = providers.SkipWhile(c => c.Name != providerName);
         }
 
+        if (feature.AllowedProviders.Any())
+        {
+            providers = providers.Where(p => feature.AllowedProviders.Contains(p.Name));
+        }
+
         var featureNameValueWithGrantedProvider = new FeatureNameValueWithGrantedProvider(name, null);
         foreach (var provider in providers)
         {

modules/feature-management/test/Volo.Abp.FeatureManagement.Application.Tests/Volo/Abp/FeatureManagement/FeatureAppService_Tests.cs

@@ -112,6 +112,72 @@ public class FeatureAppService_Tests : FeatureManagementApplicationTestBase
         Assert.Null(exception);
     }
 
+    [Fact]
+    public async Task GetAsync_Should_Not_Return_Features_With_Disallowed_Provider()
+    {
+        Login(_testData.User1Id);
+
+        var editionFeatures = await _featureAppService.GetAsync(
+            EditionFeatureValueProvider.ProviderName,
+            TestEditionIds.Regular.ToString());
+
+        editionFeatures.Groups.SelectMany(g => g.Features)
+            .ShouldNotContain(feature => feature.Name == TestFeatureDefinitionProvider.TenantOnlyFeature);
+
+        var tenantFeatures = await _featureAppService.GetAsync(
+            TenantFeatureValueProvider.ProviderName,
+            Guid.NewGuid().ToString());
+
+        tenantFeatures.Groups.SelectMany(g => g.Features)
+            .ShouldContain(feature => feature.Name == TestFeatureDefinitionProvider.TenantOnlyFeature);
+    }
+
+    [Fact]
+    public async Task GetAsync_Should_Not_Return_Orphan_Child_When_Parent_Is_Disallowed()
+    {
+        Login(_testData.User1Id);
+
+        var editionFeatures = await _featureAppService.GetAsync(
+            EditionFeatureValueProvider.ProviderName,
+            TestEditionIds.Regular.ToString());
+
+        var featureNames = editionFeatures.Groups.SelectMany(g => g.Features).Select(f => f.Name).ToList();
+        featureNames.ShouldNotContain(TestFeatureDefinitionProvider.TenantOnlyParentFeature);
+        featureNames.ShouldNotContain(TestFeatureDefinitionProvider.OrphanChildOfTenantOnly);
+
+        var tenantFeatures = await _featureAppService.GetAsync(
+            TenantFeatureValueProvider.ProviderName,
+            Guid.NewGuid().ToString());
+
+        var tenantNames = tenantFeatures.Groups.SelectMany(g => g.Features).Select(f => f.Name).ToList();
+        tenantNames.ShouldContain(TestFeatureDefinitionProvider.TenantOnlyParentFeature);
+        tenantNames.ShouldContain(TestFeatureDefinitionProvider.OrphanChildOfTenantOnly);
+    }
+
+    [Fact]
+    public async Task UpdateAsync_Should_Throw_Exception_For_Disallowed_Provider()
+    {
+        Login(_testData.User1Id);
+
+        await Assert.ThrowsAsync<AbpException>(async () =>
+        {
+            await _featureAppService.UpdateAsync(
+                EditionFeatureValueProvider.ProviderName,
+                TestEditionIds.Regular.ToString(),
+                new UpdateFeaturesDto
+                {
+                    Features = new List<UpdateFeatureDto>
+                    {
+                        new UpdateFeatureDto
+                        {
+                            Name = TestFeatureDefinitionProvider.TenantOnlyFeature,
+                            Value = true.ToString().ToLowerInvariant()
+                        }
+                    }
+                });
+        });
+    }
+
     private void Login(Guid userId)
     {
         _currentUser.Id.Returns(userId);

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

@@ -217,7 +217,118 @@ public class FeatureManager_Tests : FeatureManagementDomainTestBase
         {
             await _featureManager.SetAsync(TestFeatureDefinitionProvider.EmailSupport, "true", "UndefinedProvider", "Test");
         });
-        
+
         exception.Message.ShouldBe("Unknown feature value provider: UndefinedProvider");
     }
+
+    [Fact]
+    public async Task Set_Should_Throw_Exception_If_Provider_Not_In_AllowedProviders()
+    {
+        var exception = await Assert.ThrowsAsync<AbpException>(async () =>
+        {
+            await _featureManager.SetAsync(
+                TestFeatureDefinitionProvider.TenantOnlyFeature,
+                "true",
+                EditionFeatureValueProvider.ProviderName,
+                TestEditionIds.Enterprise.ToString());
+        });
+
+        exception.Message.ShouldContain(TestFeatureDefinitionProvider.TenantOnlyFeature);
+        exception.Message.ShouldContain(EditionFeatureValueProvider.ProviderName);
+    }
+
+    [Fact]
+    public async Task Set_Should_Allow_Setting_For_Provider_In_AllowedProviders()
+    {
+        var tenantId = Guid.NewGuid();
+
+        await _featureManager.SetAsync(
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            "true",
+            TenantFeatureValueProvider.ProviderName,
+            tenantId.ToString());
+
+        (await _featureManager.GetOrNullForTenantAsync(
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            tenantId)).ShouldBe("true");
+    }
+
+    [Fact]
+    public async Task GetAllWithProvider_Should_Not_Return_Features_With_Disallowed_Provider()
+    {
+        var editionFeatures = await _featureManager.GetAllWithProviderAsync(
+            EditionFeatureValueProvider.ProviderName,
+            TestEditionIds.Enterprise.ToString());
+
+        editionFeatures.ShouldNotContain(x => x.Name == TestFeatureDefinitionProvider.TenantOnlyFeature);
+
+        var tenantId = Guid.NewGuid();
+        await _featureManager.SetForTenantAsync(
+            tenantId,
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            "true");
+
+        var tenantFeatures = await _featureManager.GetAllWithProviderAsync(
+            TenantFeatureValueProvider.ProviderName,
+            tenantId.ToString());
+
+        tenantFeatures.ShouldContain(x =>
+            x.Name == TestFeatureDefinitionProvider.TenantOnlyFeature && x.Value == "true");
+    }
+
+    [Fact]
+    public async Task GetOrNullWithProvider_Should_Not_Read_From_Disallowed_Provider()
+    {
+        var editionId = TestEditionIds.Enterprise.ToString();
+        await _featureValueRepository.InsertAsync(new FeatureValue(
+            Guid.NewGuid(),
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            "true",
+            EditionFeatureValueProvider.ProviderName,
+            editionId));
+
+        var value = await _featureManager.GetOrNullWithProviderAsync(
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            EditionFeatureValueProvider.ProviderName,
+            editionId);
+
+        value.Value.ShouldBeNull();
+    }
+
+    [Fact]
+    public async Task GetOrNullForEdition_Should_Read_Value_For_Edition_Only_Feature()
+    {
+        var editionId = Guid.NewGuid();
+
+        await _featureManager.SetForEditionAsync(
+            editionId,
+            TestFeatureDefinitionProvider.EditionOnlyFeature,
+            "true");
+
+        (await _featureManager.GetOrNullForEditionAsync(
+            TestFeatureDefinitionProvider.EditionOnlyFeature,
+            editionId)).ShouldBe("true");
+    }
+
+    [Fact]
+    public async Task Set_Should_Not_Clear_Tenant_Value_Due_To_Stale_Disallowed_Provider_Fallback()
+    {
+        var tenantId = Guid.NewGuid();
+
+        await _featureValueRepository.InsertAsync(new FeatureValue(
+            Guid.NewGuid(),
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            "true",
+            EditionFeatureValueProvider.ProviderName,
+            null));
+
+        await _featureManager.SetForTenantAsync(
+            tenantId,
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            "true");
+
+        (await _featureManager.GetOrNullForTenantAsync(
+            TestFeatureDefinitionProvider.TenantOnlyFeature,
+            tenantId)).ShouldBe("true");
+    }
 }

modules/feature-management/test/Volo.Abp.FeatureManagement.TestBase/Volo/Abp/FeatureManagement/TestFeatureDefinitionProvider.cs

@@ -12,6 +12,10 @@ public class TestFeatureDefinitionProvider : FeatureDefinitionProvider
     public const string UserCount = "UserCount";
     public const string ProjectCount = "ProjectCount";
     public const string BackupCount = "BackupCount";
+    public const string TenantOnlyFeature = "TenantOnlyFeature";
+    public const string TenantOnlyParentFeature = "TenantOnlyParentFeature";
+    public const string OrphanChildOfTenantOnly = "OrphanChildOfTenantOnly";
+    public const string EditionOnlyFeature = "EditionOnlyFeature";
 
     public override void Define(IFeatureDefinitionContext context)
     {
@@ -22,6 +26,29 @@ public class TestFeatureDefinitionProvider : FeatureDefinitionProvider
             valueType: new ToggleStringValueType()
         );
 
+        group.AddFeature(
+            TenantOnlyFeature,
+            defaultValue: false.ToString().ToLowerInvariant(),
+            valueType: new ToggleStringValueType()
+        ).WithProviders(TenantFeatureValueProvider.ProviderName);
+
+        var tenantOnlyParent = group.AddFeature(
+            TenantOnlyParentFeature,
+            defaultValue: false.ToString().ToLowerInvariant(),
+            valueType: new ToggleStringValueType()
+        ).WithProviders(TenantFeatureValueProvider.ProviderName);
+
+        tenantOnlyParent.CreateChild(
+            OrphanChildOfTenantOnly,
+            defaultValue: false.ToString().ToLowerInvariant(),
+            valueType: new ToggleStringValueType());
+
+        group.AddFeature(
+            EditionOnlyFeature,
+            defaultValue: false.ToString().ToLowerInvariant(),
+            valueType: new ToggleStringValueType()
+        ).WithProviders(EditionFeatureValueProvider.ProviderName);
+
         var emailSupport = group.AddFeature(
             EmailSupport,
             "true",

modules/permission-management/src/Volo.Abp.PermissionManagement.Domain/Volo/Abp/PermissionManagement/PermissionManager.cs

@@ -144,7 +144,7 @@ public class PermissionManager : IPermissionManager, ISingletonDependency
         if (permission.Providers.Any() && !permission.Providers.Contains(providerName))
         {
             //TODO: BusinessException
-            throw new ApplicationException($"The permission named '{permission.Name}' has not compatible with the provider named '{providerName}'");
+            throw new ApplicationException($"The permission named '{permission.Name}' is not compatible with the provider named '{providerName}'");
         }
 
         if (!permission.MultiTenancySide.HasFlag(CurrentTenant.GetMultiTenancySide()))

modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManager.cs

@@ -73,11 +73,20 @@ public class SettingManager : ISettingManager, ISingletonDependency
 
         foreach (var setting in settingDefinitions)
         {
+            var settingProviderList = setting.Providers.Any()
+                ? providerList.Where(p => setting.Providers.Contains(p.Name)).ToList()
+                : providerList;
+
+            if (!settingProviderList.Any())
+            {
+                continue;
+            }
+
             string value = null;
 
             if (setting.IsInherited)
             {
-                foreach (var provider in providerList)
+                foreach (var provider in settingProviderList)
                 {
                     var providerValue = await provider.GetOrNullAsync(
                         setting,
@@ -91,7 +100,7 @@ public class SettingManager : ISettingManager, ISingletonDependency
             }
             else
             {
-                value = await providerList[0].GetOrNullAsync(
+                value = await settingProviderList[0].GetOrNullAsync(
                     setting,
                     providerKey
                 );
@@ -118,6 +127,11 @@ public class SettingManager : ISettingManager, ISingletonDependency
 
         var setting = await SettingDefinitionManager.GetAsync(name);
 
+        if (setting.Providers.Any() && !setting.Providers.Contains(providerName))
+        {
+            throw new AbpException($"The setting named '{name}' is not compatible with the provider named '{providerName}'");
+        }
+
         var providers = Enumerable
             .Reverse(Providers)
             .SkipWhile(p => p.Name != providerName)
@@ -188,6 +202,11 @@ public class SettingManager : ISettingManager, ISingletonDependency
             providers = providers.TakeWhile(c => c.Name == providerName);
         }
 
+        if (setting.Providers.Any())
+        {
+            providers = providers.Where(p => setting.Providers.Contains(p.Name));
+        }
+
         string value = null;
         foreach (var provider in providers)
         {

modules/setting-management/test/Volo.Abp.SettingManagement.TestBase/Volo/Abp/SettingManagement/TestSettingDefinitionProvider.cs

@@ -4,6 +4,9 @@ namespace Volo.Abp.SettingManagement;
 
 public class TestSettingDefinitionProvider : SettingDefinitionProvider
 {
+    public const string UserOnlySetting = "UserOnlySetting";
+    public const string GlobalOnlySetting = "GlobalOnlySetting";
+
     public override void Define(ISettingDefinitionContext context)
     {
         context.Add(new SettingDefinition("MySetting1"));
@@ -11,7 +14,9 @@ public class TestSettingDefinitionProvider : SettingDefinitionProvider
         context.Add(new SettingDefinition("MySetting3", "123"));
         context.Add(new SettingDefinition("MySettingWithoutInherit", isInherited: false));
         context.Add(new SettingDefinition("SettingNotSetInStore", defaultValue: "default-value"));
-
+        context.Add(new SettingDefinition(UserOnlySetting)
-
+            .WithProviders(UserSettingValueProvider.ProviderName));
+        context.Add(new SettingDefinition(GlobalOnlySetting)
+            .WithProviders(GlobalSettingValueProvider.ProviderName));
     }
 }

modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingManager_Basic_Tests.cs

@@ -10,11 +10,13 @@ public class SettingManager_Basic_Tests : SettingsTestBase
 {
     private readonly ISettingManager _settingManager;
     private readonly ISettingProvider _settingProvider;
+    private readonly ISettingManagementStore _settingManagementStore;
 
     public SettingManager_Basic_Tests()
     {
         _settingManager = GetRequiredService<ISettingManager>();
         _settingProvider = GetRequiredService<ISettingProvider>();
+        _settingManagementStore = GetRequiredService<ISettingManagementStore>();
     }
 
     [Fact]
@@ -74,4 +76,101 @@ public class SettingManager_Basic_Tests : SettingsTestBase
 
         exception.Message.ShouldBe("Unknown setting value provider: UndefinedProvider");
     }
+
+    [Fact]
+    public async Task Set_Should_Throw_Exception_If_Provider_Not_In_Providers()
+    {
+        var exception = await Assert.ThrowsAsync<AbpException>(async () =>
+        {
+            await _settingManager.SetGlobalAsync(TestSettingDefinitionProvider.UserOnlySetting, "value");
+        });
+
+        exception.Message.ShouldContain(TestSettingDefinitionProvider.UserOnlySetting);
+        exception.Message.ShouldContain(GlobalSettingValueProvider.ProviderName);
+    }
+
+    [Fact]
+    public async Task Set_Should_Allow_Setting_For_Provider_In_Providers()
+    {
+        var userId = Guid.NewGuid();
+
+        await _settingManager.SetForUserAsync(userId, TestSettingDefinitionProvider.UserOnlySetting, "value");
+
+        (await _settingManager.GetOrNullForUserAsync(
+            TestSettingDefinitionProvider.UserOnlySetting,
+            userId)).ShouldBe("value");
+    }
+
+    [Fact]
+    public async Task GetAllAsync_Should_Not_Return_Settings_With_Disallowed_Provider()
+    {
+        var userId = Guid.NewGuid();
+        await _settingManager.SetForUserAsync(userId, TestSettingDefinitionProvider.UserOnlySetting, "user-value");
+
+        var globalSettings = await _settingManager.GetAllGlobalAsync();
+        globalSettings.ShouldNotContain(x => x.Name == TestSettingDefinitionProvider.UserOnlySetting);
+
+        var userSettings = await _settingManager.GetAllForUserAsync(userId);
+        userSettings.ShouldContain(x =>
+            x.Name == TestSettingDefinitionProvider.UserOnlySetting && x.Value == "user-value");
+    }
+
+    [Fact]
+    public async Task GetOrNullForGlobal_Should_Not_Read_From_Disallowed_Provider()
+    {
+        await _settingManagementStore.SetAsync(
+            TestSettingDefinitionProvider.UserOnlySetting,
+            "stale",
+            GlobalSettingValueProvider.ProviderName,
+            null);
+
+        (await _settingManager.GetOrNullGlobalAsync(
+            TestSettingDefinitionProvider.UserOnlySetting)).ShouldBeNull();
+    }
+
+    [Fact]
+    public async Task GetAllForUser_Should_Inherit_Setting_From_Allowed_Upstream_Provider()
+    {
+        await _settingManager.SetGlobalAsync(
+            TestSettingDefinitionProvider.GlobalOnlySetting,
+            "global-value");
+
+        var userSettings = await _settingManager.GetAllForUserAsync(Guid.NewGuid());
+
+        userSettings.ShouldContain(x =>
+            x.Name == TestSettingDefinitionProvider.GlobalOnlySetting && x.Value == "global-value");
+    }
+
+    [Fact]
+    public async Task GetOrNullForUser_Should_Inherit_Value_From_Allowed_Upstream_Provider()
+    {
+        await _settingManager.SetGlobalAsync(
+            TestSettingDefinitionProvider.GlobalOnlySetting,
+            "global-value");
+
+        (await _settingManager.GetOrNullForUserAsync(
+            TestSettingDefinitionProvider.GlobalOnlySetting,
+            Guid.NewGuid())).ShouldBe("global-value");
+    }
+
+    [Fact]
+    public async Task SetForUser_Should_Not_Be_Cleared_By_Stale_Disallowed_Provider_Fallback()
+    {
+        var userId = Guid.NewGuid();
+
+        await _settingManagementStore.SetAsync(
+            TestSettingDefinitionProvider.UserOnlySetting,
+            "user-value",
+            GlobalSettingValueProvider.ProviderName,
+            null);
+
+        await _settingManager.SetForUserAsync(
+            userId,
+            TestSettingDefinitionProvider.UserOnlySetting,
+            "user-value");
+
+        (await _settingManager.GetOrNullForUserAsync(
+            TestSettingDefinitionProvider.UserOnlySetting,
+            userId)).ShouldBe("user-value");
+    }
 }