Allow different type of filters to share the same Alias

src/Microsoft.FeatureManagement/FeatureManager.cs

@@ -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);
+                        }
+                        else
+                        {
+                            filter = GetFeatureFilterMetadata(featureFilterConfiguration.Name);
+                        }
 
                         if (filter == null)
                         {
@@ -163,14 +173,14 @@ private async Task<bool> IsEnabledAsync<TContext>(string feature, TContext appCo
                             Parameters = featureFilterConfiguration.Parameters
                         };
 
+                        BindSettings(filter, context, filterIndex);
+
                         //
                         // IContextualFeatureFilter
                         if (useAppContext)
                         {
                             ContextualFeatureFilterEvaluator contextualFilter = GetContextualFeatureFilter(featureFilterConfiguration.Name, typeof(TContext));
 
-                            BindSettings(filter, context, filterIndex);
-
                             if (contextualFilter != null &&
                                 await contextualFilter.EvaluateAsync(context, appContext).ConfigureAwait(false) == targetEvaluation)
                             {
@@ -184,9 +194,8 @@ await contextualFilter.EvaluateAsync(context, appContext).ConfigureAwait(false)
                         // IFeatureFilter
                         if (filter is IFeatureFilter featureFilter)
                         {
-                            BindSettings(filter, context, filterIndex);
-
-                            if (await featureFilter.EvaluateAsync(context).ConfigureAwait(false) == targetEvaluation) {
+                            if (await featureFilter.EvaluateAsync(context).ConfigureAwait(false) == targetEvaluation)
+                            {
                                 enabled = targetEvaluation;
 
                                 break;
@@ -267,48 +276,39 @@ private void BindSettings(IFeatureFilterMetadata filter, FeatureFilterEvaluation
             context.Settings = settings;
         }
 
-        private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName)
+        private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName, Type appContextType = null)
         {
-            const string filterSuffix = "filter";
-
             IFeatureFilterMetadata filter = _filterMetadataCache.GetOrAdd(
-                filterName,
+                $"{filterName}{Environment.NewLine}{appContextType?.FullName}",
                 (_) => {
 
                     IEnumerable<IFeatureFilterMetadata> matchingFilters = _featureFilters.Where(f =>
                     {
-                        Type t = f.GetType();
-
-                        string name = ((FilterAliasAttribute)Attribute.GetCustomAttribute(t, typeof(FilterAliasAttribute)))?.Alias;
+                        Type filterType = f.GetType();
 
-                        if (name == null)
+                        if (!IsMatchingName(filterType, filterName))
                         {
-                            name = t.Name.EndsWith(filterSuffix, StringComparison.OrdinalIgnoreCase) ? t.Name.Substring(0, t.Name.Length - filterSuffix.Length) : t.Name;
+                            return false;
                         }
 
-                        //
-                        // 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('.'))
+                        if (appContextType == null)
                         {
-                            //
-                            // The configured filter name is namespaced. It must be an exact match.
-                            return string.Equals(name, filterName, StringComparison.OrdinalIgnoreCase);
+                            return (f is IFeatureFilter);
                         }
-                        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);
-                        }
+                        return ContextualFeatureFilterEvaluator.IsContextualFilter(f, appContextType);
                     });
 
                     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();
@@ -318,6 +318,37 @@ private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName)
             return filter;
         }
 
+        private bool IsMatchingName(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 ContextualFeatureFilterEvaluator GetContextualFeatureFilter(string filterName, Type appContextType)
         {
             if (appContextType == null)
@@ -329,11 +360,14 @@ private ContextualFeatureFilterEvaluator GetContextualFeatureFilter(string filte
                 $"{filterName}{Environment.NewLine}{appContextType.FullName}",
                 (_) => {
 
-                    IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName);
+                    IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName, appContextType);
+
+                    if (metadata == null)
+                    {
+                        return null;
+                    }
 
-                    return ContextualFeatureFilterEvaluator.IsContextualFilter(metadata, appContextType) ?
-                        new ContextualFeatureFilterEvaluator(metadata, appContextType) :
-                        null;
+                    return new ContextualFeatureFilterEvaluator(metadata, appContextType);
                 }
             );
 

tests/Tests.FeatureManagement/FeatureManagement.cs

@@ -85,6 +85,120 @@ 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>();
+
+            var appContext = new AppContext();
+
+            var dummyContext = new DummyContext();
+
+            var targetingContext = new TargetingContext();
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName));
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, appContext));
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, dummyContext));
+
+            Assert.True(await featureManager.IsEnabledAsync(featureName, targetingContext));
+
+            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>();
+
+            var 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 +289,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);
+        }
+    }
+}