From ab128947c7e6adc2a2a1aa7a5181ca1004a2132f Mon Sep 17 00:00:00 2001
From: maliming <malimings@gmail.com>
Date: Fri, 22 May 2026 12:00:37 +0800
Subject: [PATCH] Address Copilot review

- SettingManager.GetAllAsync / FeatureManager.GetAllWithProviderAsync: drop top-level continue and rely on the provider chain filter so allowed upstream providers can still be read via inheritance (e.g. GetAllForUserAsync now returns a Global-only setting)
- FeatureAppService.GetAsync: switch includedFeatures to HashSet for O(1) parent lookup
---
 .../Volo/Abp/FeatureManagement/FeatureAppService.cs |  2 +-
 .../Volo/Abp/FeatureManagement/FeatureManager.cs    | 10 +++++-----
 .../Volo/Abp/SettingManagement/SettingManager.cs    | 10 +++++-----
 .../SettingManagement/SettingManager_Basic_Tests.cs | 13 +++++++++++++
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/modules/feature-management/src/Volo.Abp.FeatureManagement.Application/Volo/Abp/FeatureManagement/FeatureAppService.cs b/modules/feature-management/src/Volo.Abp.FeatureManagement.Application/Volo/Abp/FeatureManagement/FeatureAppService.cs
index ceb9f70441..dd139e9b61 100644
--- a/modules/feature-management/src/Volo.Abp.FeatureManagement.Application/Volo/Abp/FeatureManagement/FeatureAppService.cs
+++ b/modules/feature-management/src/Volo.Abp.FeatureManagement.Application/Volo/Abp/FeatureManagement/FeatureAppService.cs
@@ -38,7 +38,7 @@ public class FeatureAppService : FeatureManagementAppServiceBase, IFeatureAppSer
         {
             var groupDto = CreateFeatureGroupDto(group);
 
-            var includedFeatures = new List<FeatureDefinition>();
+            var includedFeatures = new HashSet<FeatureDefinition>();
             foreach (var featureDefinition in group.GetFeaturesWithChildren())
             {
                 if (providerName == TenantFeatureValueProvider.ProviderName &&
diff --git a/modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManager.cs b/modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManager.cs
index 979dba5fa7..e368eefe76 100644
--- a/modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManager.cs
+++ b/modules/feature-management/src/Volo.Abp.FeatureManagement.Domain/Volo/Abp/FeatureManagement/FeatureManager.cs
@@ -93,15 +93,15 @@ public class FeatureManager : IFeatureManager, ISingletonDependency
 
         foreach (var feature in featureDefinitions)
         {
-            if (feature.AllowedProviders.Any() && !feature.AllowedProviders.Contains(providerName))
-            {
-                continue;
-            }
-
             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 featureProviderList)
             {
diff --git a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManager.cs b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManager.cs
index 828503e169..c8be7d49f4 100644
--- a/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManager.cs
+++ b/modules/setting-management/src/Volo.Abp.SettingManagement.Domain/Volo/Abp/SettingManagement/SettingManager.cs
@@ -73,15 +73,15 @@ public class SettingManager : ISettingManager, ISingletonDependency
 
         foreach (var setting in settingDefinitions)
         {
-            if (setting.Providers.Any() && !setting.Providers.Contains(providerName))
-            {
-                continue;
-            }
-
             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)
diff --git a/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingManager_Basic_Tests.cs b/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingManager_Basic_Tests.cs
index 17873338d2..075d4e65f2 100644
--- a/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingManager_Basic_Tests.cs
+++ b/modules/setting-management/test/Volo.Abp.SettingManagement.Tests/Volo/Abp/SettingManagement/SettingManager_Basic_Tests.cs
@@ -128,6 +128,19 @@ public class SettingManager_Basic_Tests : SettingsTestBase
             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()
     {