Skip to content

Allow for both IFeatureFilter and IContextualFeatureFilter with same alias #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Allow for both IFeatureFilter and IContextualFeatureFilter with same alias #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Microsoft.FeatureManagement/FeatureManagementOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,11 @@ public class FeatureManagementOptions
/// The default value is true.
/// </summary>
public bool IgnoreMissingFeatures { get; set; } = true;

/// <summary>
/// Gets or sets a value indicating whether to allow for the same alias name to be used for a <see cref="IFeatureFilter" /> and a
/// <see cref="IContextualFeatureFilter{TContext}" />. Default is false.
/// </summary>
public bool AllowDuplicateContextualAlias { get; set; }
}
}
203 changes: 128 additions & 75 deletions src/Microsoft.FeatureManagement/FeatureManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ namespace Microsoft.FeatureManagement
/// </summary>
class FeatureManager : IFeatureManager
{
private delegate Task<bool> FilterEvaluator(FeatureFilterEvaluationContext context, object appContext);
private readonly IFeatureDefinitionProvider _featureDefinitionProvider;
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>, FilterEvaluator> _evaluatorCache;
private readonly FeatureManagementOptions _options;

public FeatureManager(
Expand All @@ -35,8 +35,7 @@ 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>();
_evaluatorCache = new ConcurrentDictionary<ValueTuple<string, Type>, FilterEvaluator>();
_options = options?.Value ?? throw new ArgumentNullException(nameof(options));
}

Expand All @@ -58,6 +57,47 @@ public async IAsyncEnumerable<string> 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<bool> IsEnabledAsync<TContext>(string feature, TContext appContext, bool useAppContext)
{
foreach (ISessionManager sessionManager in _sessionManagers)
Expand Down Expand Up @@ -92,7 +132,8 @@ private async Task<bool> IsEnabledAsync<TContext>(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)
{
Expand All @@ -116,26 +157,9 @@ private async Task<bool> IsEnabledAsync<TContext>(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;
}
}
Expand Down Expand Up @@ -163,77 +187,106 @@ private async Task<bool> IsEnabledAsync<TContext>(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<T> implementations, so long as <T> is assignable to only 1
// discovered contextual filter (so no IContextFeatureFilter<T> and IContextFeatureFilter<interface of T>).
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<IFeatureFilterMetadata> 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;
});
}
}
}