Allow different type of filters to share the same Alias

src/Microsoft.FeatureManagement/FeatureManager.cs

@@ -25,8 +25,8 @@ class FeatureManager : IFeatureManager, IDisposable
         private readonly IEnumerable<IFeatureFilterMetadata> _featureFilters;
         private readonly IEnumerable<ISessionManager> _sessionManagers;
         private readonly ILogger _logger;
-        private readonly ConcurrentDictionary<string, IFeatureFilterMetadata> _filterMetadataCache;
-        private readonly ConcurrentDictionary<string, ContextualFeatureFilterEvaluator> _contextualFeatureFilterCache;
+        private readonly ConcurrentDictionary<ValueTuple<string, Type>, IFeatureFilterMetadata> _filterMetadataCache;
+        private readonly ConcurrentDictionary<ValueTuple<string, Type>, ContextualFeatureFilterEvaluator> _contextualFeatureFilterCache;
         private readonly FeatureManagementOptions _options;
         private readonly IMemoryCache _parametersCache;
 
@@ -48,8 +48,8 @@ public FeatureManager(
             _featureFilters = featureFilters ?? throw new ArgumentNullException(nameof(featureFilters));
             _sessionManagers = sessionManagers ?? throw new ArgumentNullException(nameof(sessionManagers));
             _logger = loggerFactory.CreateLogger<FeatureManager>();
-            _filterMetadataCache = new ConcurrentDictionary<string, IFeatureFilterMetadata>();
-            _contextualFeatureFilterCache = new ConcurrentDictionary<string, ContextualFeatureFilterEvaluator>();
+            _filterMetadataCache = new ConcurrentDictionary<ValueTuple<string, Type>, IFeatureFilterMetadata>();
+            _contextualFeatureFilterCache = new ConcurrentDictionary<ValueTuple<string, Type>, ContextualFeatureFilterEvaluator>();
             _options = options?.Value ?? throw new ArgumentNullException(nameof(options));
             _parametersCache = new MemoryCache(new MemoryCacheOptions());
         }
@@ -141,7 +141,17 @@ private async Task<bool> IsEnabledAsync<TContext>(string feature, TContext appCo
                             continue;
                         }
 
-                        IFeatureFilterMetadata filter = GetFeatureFilterMetadata(featureFilterConfiguration.Name);
+                        IFeatureFilterMetadata filter;
+
+                        if (useAppContext)
+                        {
+                            filter = GetFeatureFilterMetadata(featureFilterConfiguration.Name, typeof(TContext)) ??
+                                     GetFeatureFilterMetadata(featureFilterConfiguration.Name, null);
+                        }
+                        else
+                        {
+                            filter = GetFeatureFilterMetadata(featureFilterConfiguration.Name, null);
+                        }
 
                         if (filter == null)
                         {
@@ -165,7 +175,7 @@ private async Task<bool> IsEnabledAsync<TContext>(string feature, TContext appCo
 
                         //
                         // IContextualFeatureFilter
-                        if (useAppContext)
+                        if (useAppContext && ContextualFeatureFilterEvaluator.IsContextualFilter(filter, typeof(TContext)))
                         {
                             ContextualFeatureFilterEvaluator contextualFilter = GetContextualFeatureFilter(featureFilterConfiguration.Name, typeof(TContext));
 
@@ -267,48 +277,72 @@ private void BindSettings(IFeatureFilterMetadata filter, FeatureFilterEvaluation
             context.Settings = settings;
         }
 
-        private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName)
+        private bool IsFilterNameMatched(Type filterType, string filterName)
         {
             const string filterSuffix = "filter";
 
+            string name = ((FilterAliasAttribute)Attribute.GetCustomAttribute(filterType, typeof(FilterAliasAttribute)))?.Alias;
+
+            if (name == null)
+            {
+                name = filterType.Name.EndsWith(filterSuffix, StringComparison.OrdinalIgnoreCase) ? filterType.Name.Substring(0, filterType.Name.Length - filterSuffix.Length) : filterType.Name;
+            }
+
+            //
+            // Feature filters can have namespaces in their alias
+            // If a feature is configured to use a filter without a namespace such as 'MyFilter', then it can match 'MyOrg.MyProduct.MyFilter' or simply 'MyFilter'
+            // If a feature is configured to use a filter with a namespace such as 'MyOrg.MyProduct.MyFilter' then it can only match 'MyOrg.MyProduct.MyFilter' 
+            if (filterName.Contains('.'))
+            {
+                //
+                // The configured filter name is namespaced. It must be an exact match.
+                return string.Equals(name, filterName, StringComparison.OrdinalIgnoreCase);
+            }
+            else
+            {
+                //
+                // We take the simple name of a filter, E.g. 'MyFilter' for 'MyOrg.MyProduct.MyFilter'
+                string simpleName = name.Contains('.') ? name.Split('.').Last() : name;
+
+                return string.Equals(simpleName, filterName, StringComparison.OrdinalIgnoreCase);
+            }
+        }
+
+        private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName, Type appContextType)
+        {
             IFeatureFilterMetadata filter = _filterMetadataCache.GetOrAdd(
-                filterName,
+                (filterName, appContextType),
                 (_) => {
 
                     IEnumerable<IFeatureFilterMetadata> matchingFilters = _featureFilters.Where(f =>
                     {
-                        Type t = f.GetType();
+                        Type filterType = f.GetType();
 
-                        string name = ((FilterAliasAttribute)Attribute.GetCustomAttribute(t, typeof(FilterAliasAttribute)))?.Alias;
-
-                        if (name == null)
+                        if (appContextType == null)
                         {
-                            name = t.Name.EndsWith(filterSuffix, StringComparison.OrdinalIgnoreCase) ? t.Name.Substring(0, t.Name.Length - filterSuffix.Length) : t.Name;
-                        }
-
-                        //
-                        // Feature filters can have namespaces in their alias
-                        // If a feature is configured to use a filter without a namespace such as 'MyFilter', then it can match 'MyOrg.MyProduct.MyFilter' or simply 'MyFilter'
-                        // If a feature is configured to use a filter with a namespace such as 'MyOrg.MyProduct.MyFilter' then it can only match 'MyOrg.MyProduct.MyFilter' 
-                        if (filterName.Contains('.'))
-                        {
-                            //
-                            // The configured filter name is namespaced. It must be an exact match.
-                            return string.Equals(name, filterName, StringComparison.OrdinalIgnoreCase);
+                            return (f is IFeatureFilter) && IsFilterNameMatched(filterType, filterName);
                         }
                         else
                         {
-                            //
-                            // We take the simple name of a filter, E.g. 'MyFilter' for 'MyOrg.MyProduct.MyFilter'
-                            string simpleName = name.Contains('.') ? name.Split('.').Last() : name;
+                            IEnumerable<Type> contextualFilterInterfaces = filterType.GetInterfaces().Where(
+                                i => i.IsGenericType &&
+                                     i.GetGenericTypeDefinition().IsAssignableFrom(typeof(IContextualFeatureFilter<>)) &&
+                                     i.GetGenericArguments()[0].IsAssignableFrom(appContextType));
 
-                            return string.Equals(simpleName, filterName, StringComparison.OrdinalIgnoreCase);
+                            return contextualFilterInterfaces.Any() && IsFilterNameMatched(filterType, filterName);
                         }
                     });
 
                     if (matchingFilters.Count() > 1)
                     {
-                        throw new FeatureManagementException(FeatureManagementError.AmbiguousFeatureFilter, $"Multiple feature filters match the configured filter named '{filterName}'.");
+                        if (appContextType == null)
+                        {
+                            throw new FeatureManagementException(FeatureManagementError.AmbiguousFeatureFilter, $"Multiple feature filters match the configured filter named '{filterName}'.");
+                        }
+                        else
+                        {
+                            throw new FeatureManagementException(FeatureManagementError.AmbiguousFeatureFilter, $"Multiple contextual feature filters match the configured filter named '{filterName}' and context type '{appContextType}'.");
+                        }
                     }
 
                     return matchingFilters.FirstOrDefault();
@@ -326,14 +360,12 @@ private ContextualFeatureFilterEvaluator GetContextualFeatureFilter(string filte
             }
 
             ContextualFeatureFilterEvaluator filter = _contextualFeatureFilterCache.GetOrAdd(
-                $"{filterName}{Environment.NewLine}{appContextType.FullName}",
+                (filterName, appContextType),
                 (_) => {
 
-                    IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName);
+                    IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName, appContextType);
 
-                    return ContextualFeatureFilterEvaluator.IsContextualFilter(metadata, appContextType) ?
-                        new ContextualFeatureFilterEvaluator(metadata, appContextType) :
-                        null;
+                    return new ContextualFeatureFilterEvaluator(metadata, appContextType);
                 }
             );
 

tests/Tests.FeatureManagement/FeatureManagement.cs

@@ -85,6 +85,116 @@ public async Task ReadsOnlyFeatureManagementSection()
             }
         }
 
+        [Fact]
+        public async Task AllowDuplicatedFilterAlias()
+        {
+            const string duplicatedFilterName = "DuplicatedFilterName";
+
+            string featureName = Enum.GetName(typeof(Features), Features.FeatureUsesFiltersWithDuplicatedAlias);
+
+            IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();
+
+            var services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
+                .AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>()
+                .AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>()
+                .AddFeatureFilter<PercentageFilter>();
+
+            ServiceProvider serviceProvider = services.BuildServiceProvider();
+
+            IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            AppContext appContext = new AppContext();
+
+            DummyContext dummyContext = new DummyContext();
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName));
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, appContext));
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, dummyContext));
+
+            services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
+                .AddFeatureFilter<PercentageFilter>();
+
+            serviceProvider = services.BuildServiceProvider();
+
+            featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, dummyContext));
+
+            services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
+                .AddFeatureFilter<DuplicatedAliasFeatureFilter2>()
+                .AddFeatureFilter<PercentageFilter>();
+
+            serviceProvider = services.BuildServiceProvider();
+
+            featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            FeatureManagementException ex = await Assert.ThrowsAsync<FeatureManagementException>(
+                async () =>
+                {
+                    await featureManager.IsEnabledAsync(featureName);
+                });
+
+            Assert.Equal($"Multiple feature filters match the configured filter named '{duplicatedFilterName}'.", ex.Message);
+
+            services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>()
+                .AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext2>()
+                .AddFeatureFilter<PercentageFilter>();
+
+            serviceProvider = services.BuildServiceProvider();
+
+            featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            ex = await Assert.ThrowsAsync<FeatureManagementException>(
+                async () =>
+                {
+                    await featureManager.IsEnabledAsync(featureName, dummyContext);
+                });
+
+            Assert.Equal($"Multiple contextual feature filters match the configured filter named '{duplicatedFilterName}' and context type '{typeof(DummyContext)}'.", ex.Message);
+
+            services = new ServiceCollection();
+
+            services
+                .AddSingleton(config)
+                .AddFeatureManagement()
+                .AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>()
+                .AddFeatureFilter<PercentageFilter>();
+
+            serviceProvider = services.BuildServiceProvider();
+
+            featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
+
+            ex = await Assert.ThrowsAsync<FeatureManagementException>(
+                async () =>
+                {
+                    await featureManager.IsEnabledAsync(featureName);
+                });
+
+            Assert.Equal($"The feature filter '{duplicatedFilterName}' specified for feature '{featureName}' was not found.", ex.Message);
+        }
+
         [Fact]
         public async Task CustomFilterContextualTargetingWithNullSetting()
         {
@@ -175,7 +285,7 @@ public async Task Percentage()
                 }
             }
 
-            Assert.True(enabledCount > 0 && enabledCount < 10);
+            Assert.True(enabledCount >= 0 && enabledCount < 10);
         }
 
         [Fact]

tests/Tests.FeatureManagement/Features.cs

@@ -13,6 +13,7 @@ enum Features
         ConditionalFeature2,
         ContextualFeature,
         AnyFilterFeature,
-        AllFilterFeature
+        AllFilterFeature,
+        FeatureUsesFiltersWithDuplicatedAlias
     }
 }

tests/Tests.FeatureManagement/FiltersWithDuplicatedAlias.cs

@@ -0,0 +1,73 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+//
+using Microsoft.FeatureManagement;
+using System.Threading.Tasks;
+
+namespace Tests.FeatureManagement
+{
+    interface IDummyContext
+    {
+        string DummyProperty { get; set; }
+    }
+
+    class DummyContext : IDummyContext
+    {
+        public string DummyProperty { get; set; }
+    }
+
+    [FilterAlias(Alias)]
+    class DuplicatedAliasFeatureFilter1 : IFeatureFilter
+    {
+        private const string Alias = "DuplicatedFilterName";
+
+        public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
+        {
+            return Task.FromResult(true);
+        }
+    }
+
+    [FilterAlias(Alias)]
+    class DuplicatedAliasFeatureFilter2 : IFeatureFilter
+    {
+        private const string Alias = "DuplicatedFilterName";
+
+        public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
+        {
+            return Task.FromResult(true);
+        }
+    }
+
+    [FilterAlias(Alias)]
+    class ContextualDuplicatedAliasFeatureFilterWithAccountContext : IContextualFeatureFilter<IAccountContext>
+    {
+        private const string Alias = "DuplicatedFilterName";
+
+        public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IAccountContext accountContext)
+        {
+            return Task.FromResult(true);
+        }
+    }
+
+    [FilterAlias(Alias)]
+    class ContextualDuplicatedAliasFeatureFilterWithDummyContext1 : IContextualFeatureFilter<IDummyContext>
+    {
+        private const string Alias = "DuplicatedFilterName";
+
+        public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IDummyContext dummyContext)
+        {
+            return Task.FromResult(true);
+        }
+    }
+
+    [FilterAlias(Alias)]
+    class ContextualDuplicatedAliasFeatureFilterWithDummyContext2 : IContextualFeatureFilter<IDummyContext>
+    {
+        private const string Alias = "DuplicatedFilterName";
+
+        public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IDummyContext dummyContext)
+        {
+            return Task.FromResult(true);
+        }
+    }
+}

tests/Tests.FeatureManagement/appsettings.json

@@ -9,6 +9,20 @@
   "FeatureManagement": {
     "OnTestFeature": true,
     "OffTestFeature": false,
+    "FeatureUsesFiltersWithDuplicatedAlias": {
+      "RequirementType": "all",
+      "EnabledFor": [
+        {
+          "Name": "DuplicatedFilterName"
+        },
+        {
+          "Name": "Percentage",
+          "Parameters": {
+            "Value":  100
+          }
+        }
+      ]
+    },
     "TargetingTestFeature": {
       "EnabledFor": [
         {