diff --git a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs index e425c054..6ca36bb6 100644 --- a/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs +++ b/src/Microsoft.FeatureManagement/FeatureManagementOptions.cs @@ -21,5 +21,11 @@ public class FeatureManagementOptions /// The default value is true. /// public bool IgnoreMissingFeatures { get; set; } = true; + + /// + /// Gets or sets a value indicating whether to allow for the same alias name to be used for a and a + /// . Default is false. + /// + public bool AllowDuplicateContextualAlias { get; set; } } } diff --git a/src/Microsoft.FeatureManagement/FeatureManager.cs b/src/Microsoft.FeatureManagement/FeatureManager.cs index 097af3a2..398599cf 100644 --- a/src/Microsoft.FeatureManagement/FeatureManager.cs +++ b/src/Microsoft.FeatureManagement/FeatureManager.cs @@ -16,12 +16,12 @@ namespace Microsoft.FeatureManagement /// class FeatureManager : IFeatureManager { + private delegate Task FilterEvaluator(FeatureFilterEvaluationContext context, object appContext); private readonly IFeatureDefinitionProvider _featureDefinitionProvider; private readonly IEnumerable _featureFilters; private readonly IEnumerable _sessionManagers; private readonly ILogger _logger; - private readonly ConcurrentDictionary _filterMetadataCache; - private readonly ConcurrentDictionary _contextualFeatureFilterCache; + private readonly ConcurrentDictionary, FilterEvaluator> _evaluatorCache; private readonly FeatureManagementOptions _options; public FeatureManager( @@ -35,8 +35,7 @@ public FeatureManager( _featureFilters = featureFilters ?? throw new ArgumentNullException(nameof(featureFilters)); _sessionManagers = sessionManagers ?? throw new ArgumentNullException(nameof(sessionManagers)); _logger = loggerFactory.CreateLogger(); - _filterMetadataCache = new ConcurrentDictionary(); - _contextualFeatureFilterCache = new ConcurrentDictionary(); + _evaluatorCache = new ConcurrentDictionary, FilterEvaluator>(); _options = options?.Value ?? throw new ArgumentNullException(nameof(options)); } @@ -58,6 +57,47 @@ public async IAsyncEnumerable GetFeatureNamesAsync() } } + private static bool IsFilterNameMatch(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' + int dotIndex = name.LastIndexOf('.'); + string simpleName = dotIndex != -1 ? name.Substring(dotIndex + 1) : name; + return string.Equals(simpleName, filterName, StringComparison.OrdinalIgnoreCase); + } + } + + private static ContextualFeatureFilterEvaluator GetContextualFeatureFilter(IFeatureFilterMetadata metadata, Type appContextType) + { + if (appContextType == null) + { + throw new ArgumentNullException(nameof(appContextType)); + } + + return ContextualFeatureFilterEvaluator.IsContextualFilter(metadata, appContextType) + ? new ContextualFeatureFilterEvaluator(metadata, appContextType) : null; + } + private async Task IsEnabledAsync(string feature, TContext appContext, bool useAppContext) { foreach (ISessionManager sessionManager in _sessionManagers) @@ -92,7 +132,8 @@ private async Task IsEnabledAsync(string feature, TContext appCo foreach (FeatureFilterConfiguration featureFilterConfiguration in featureDefinition.EnabledFor) { - IFeatureFilterMetadata filter = GetFeatureFilterMetadata(featureFilterConfiguration.Name); + FilterEvaluator filter = GetFeatureFilterEvaluator( + featureFilterConfiguration.Name, useAppContext ? typeof(TContext) : null); if (filter == null) { @@ -116,26 +157,9 @@ private async Task IsEnabledAsync(string feature, TContext appCo Parameters = featureFilterConfiguration.Parameters }; - // - // IContextualFeatureFilter - if (useAppContext) + enabled = await filter.Invoke(context, appContext).ConfigureAwait(false); + if (enabled) { - ContextualFeatureFilterEvaluator contextualFilter = GetContextualFeatureFilter(featureFilterConfiguration.Name, typeof(TContext)); - - if (contextualFilter != null && await contextualFilter.EvaluateAsync(context, appContext).ConfigureAwait(false)) - { - enabled = true; - - break; - } - } - - // - // IFeatureFilter - if (filter is IFeatureFilter featureFilter && await featureFilter.EvaluateAsync(context).ConfigureAwait(false)) - { - enabled = true; - break; } } @@ -163,77 +187,106 @@ private async Task IsEnabledAsync(string feature, TContext appCo return enabled; } - private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName) + private FilterEvaluator GetFeatureFilterEvaluator( + string filterName, Type appContextType) { - const string filterSuffix = "filter"; + // + // We will support having multiple filters with the same alias if they all implement different + // IFeatureFilterMetadata interfaces. More explicitly, for a given filter alias, you can have: + // 0 or 1 IFeatureFilter implementations + // 0 or N IContextualFeatureFilter implementations, so long as is assignable to only 1 + // discovered contextual filter (so no IContextFeatureFilter and IContextFeatureFilter). + return _evaluatorCache.GetOrAdd((filterName, appContextType), key => + { + static void ThrowAmbiguousFeatureFilter(string filterName, Type appContextType = null) + { + if (appContextType is null) + { + throw new FeatureManagementException( + FeatureManagementError.AmbiguousFeatureFilter, + $"Multiple feature filters match the configured filter named '{filterName}'."); + } - IFeatureFilterMetadata filter = _filterMetadataCache.GetOrAdd( - filterName, - (_) => { + throw new FeatureManagementException( + FeatureManagementError.AmbiguousFeatureFilter, + $"Multiple contextual feature filters match the configured filter named '{filterName}'" + + $" and app context '{appContextType}'."); + } - IEnumerable matchingFilters = _featureFilters.Where(f => + (string filterName, Type appContextType) = key; + bool foundOneMatch = false; + IFeatureFilter filter = null; + ContextualFeatureFilterEvaluator contextualEvaluator = null; + foreach (IFeatureFilterMetadata metadata in _featureFilters) + { + Type t = metadata.GetType(); + if (!IsFilterNameMatch(t, filterName)) { - Type t = f.GetType(); + continue; + } - string name = ((FilterAliasAttribute)Attribute.GetCustomAttribute(t, typeof(FilterAliasAttribute)))?.Alias; + if (!_options.AllowDuplicateContextualAlias && foundOneMatch) + { + // Retain existing behavior of throwing when two aliases match, regardless of if the + // contextual evaluation is applicable. + ThrowAmbiguousFeatureFilter(filterName); + } - if (name == null) + foundOneMatch = true; + if (metadata is IFeatureFilter f) + { + if (filter is object) { - name = t.Name.EndsWith(filterSuffix, StringComparison.OrdinalIgnoreCase) ? t.Name.Substring(0, t.Name.Length - filterSuffix.Length) : t.Name; + // More than 1 matching IFeatureFilter. + ThrowAmbiguousFeatureFilter(filterName); } - // - // 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('.')) + _logger.LogDebug("Filter {FilterType} matched {FilterName} as IFeatureFilter.", t, filterName); + filter = f; + continue; + } + + if (appContextType is object) + { + ContextualFeatureFilterEvaluator evaluator = GetContextualFeatureFilter( + metadata, appContextType); + if (contextualEvaluator is null) { - // - // The configured filter name is namespaced. It must be an exact match. - return string.Equals(name, filterName, StringComparison.OrdinalIgnoreCase); + _logger.LogDebug( + "Filter {FilterType} matched {FilterName} as IContextualFeatureFilter.", t, filterName); + contextualEvaluator = evaluator; } - else + else if (evaluator is object) { - // - // 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); + // More than 1 matching IContextualFeatureFilter + ThrowAmbiguousFeatureFilter(filterName, appContextType); } - }); - if (matchingFilters.Count() > 1) - { - throw new FeatureManagementException(FeatureManagementError.AmbiguousFeatureFilter, $"Multiple feature filters match the configured filter named '{filterName}'."); + continue; } - return matchingFilters.FirstOrDefault(); + _logger.LogTrace("Filter {FilterType} matched {FilterName} but not app context.", t, filterName); } - ); - return filter; - } - - private ContextualFeatureFilterEvaluator GetContextualFeatureFilter(string filterName, Type appContextType) - { - if (appContextType == null) - { - throw new ArgumentNullException(nameof(appContextType)); - } - - ContextualFeatureFilterEvaluator filter = _contextualFeatureFilterCache.GetOrAdd( - $"{filterName}{Environment.NewLine}{appContextType.FullName}", - (_) => { - - IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName); - - return ContextualFeatureFilterEvaluator.IsContextualFilter(metadata, appContextType) ? - new ContextualFeatureFilterEvaluator(metadata, appContextType) : - null; + if (contextualEvaluator is object) + { + // Only present when appContextType is not null. + return (context, appContext) => contextualEvaluator.EvaluateAsync(context, appContext); + } + else if (filter is object) + { + // When appContextType is null or there was no matching contextual filter. + return (context, appContext) => filter.EvaluateAsync(context); + } + else if (foundOneMatch) + { + // This block means we found an incompatible contextual evaluator. To retain the existing + // behavior, we need to return a constant false evaluation here. + return (context, appContext) => Task.FromResult(false); } - ); - return filter; + return null; + }); } } }