Skip to content

Commit 7ac36fd

Browse files
zhiyuanliang-msjimmyca15Copilotlinglingye001
authored
Merge main to release/v4 (#600)
* Remove FeatureFilterEvaluationContext.ParametersObject in favor of Settings. (#599) Co-authored-by: Copilot <copilot@github.com> * Skip parameter binding if ParamemtersObject provided (#597) * Skip parameter binding if the provider has already supplied a parameters object * update * update --------- Co-authored-by: Zhiyuan Liang <zhiyuanliang@microsoft.com> --------- Co-authored-by: Jimmy Campbell <jimmyca@microsoft.com> Co-authored-by: Copilot <copilot@github.com> Co-authored-by: linglingye001 <143174321+linglingye001@users.noreply.github.com>
2 parents 8971d2a + 8faed1a commit 7ac36fd

6 files changed

Lines changed: 153 additions & 37 deletions

File tree

‎src/Microsoft.FeatureManagement/FeatureFilterEvaluationContext.cs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,12 @@ public class FeatureFilterEvaluationContext
2222
public IConfiguration Parameters { get; set; }
2323

2424
/// <summary>
25-
/// The settings provided for the feature filter to use when evaluating whether the feature should be enabled. This property takes precedence over <see cref="Settings"/> and <see cref="Parameters"/> if both are provided.
26-
/// </summary>
27-
public object ParametersObject { get; set; }
28-
29-
/// <summary>
30-
/// A settings object, if any, that has been pre-bound from <see cref="Parameters"/>.
31-
/// The settings are made available for <see cref="IFeatureFilter"/>s that implement <see cref="IFilterParametersBinder"/>.
25+
/// A settings object, if any, provided for the feature filter to use when evaluating whether the feature should be enabled.
26+
/// This property is populated in two cases:
27+
/// <list type="bullet">
28+
/// <item>For features that provide parameters as an object, via <see cref="FeatureFilterConfiguration.ParametersObject"/>.</item>
29+
/// <item>For <see cref="IFeatureFilter"/>s that implement <see cref="IFilterParametersBinder"/>.</item>
30+
/// </list>
3231
/// </summary>
3332
public object Settings { get; set; }
3433

‎src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,16 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
4949
throw new ArgumentNullException(nameof(context));
5050
}
5151

52-
//
53-
// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
54-
PercentageFilterSettings settings;
55-
56-
if (context.ParametersObject != null && !(context.ParametersObject is PercentageFilterSettings))
52+
if (context.Settings != null && !(context.Settings is PercentageFilterSettings))
5753
{
5854
throw new ArgumentException(
59-
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(PercentageFilterSettings)}'.",
60-
nameof(context.ParametersObject));
55+
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.Settings)} value of type '{context.Settings.GetType()}', but expected '{typeof(PercentageFilterSettings)}'.",
56+
nameof(context.Settings));
6157
}
6258

63-
settings = (PercentageFilterSettings)context.ParametersObject ?? (PercentageFilterSettings)context.Settings ?? (PercentageFilterSettings)BindParameters(context.Parameters);
59+
//
60+
// Check if prebound settings available, otherwise bind from parameters.
61+
PercentageFilterSettings settings = (PercentageFilterSettings)context.Settings ?? (PercentageFilterSettings)BindParameters(context.Parameters);
6462

6563
bool result = true;
6664

‎src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,16 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
7070
throw new ArgumentNullException(nameof(context));
7171
}
7272

73-
//
74-
// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
75-
TimeWindowFilterSettings settings;
76-
77-
if (context.ParametersObject != null && !(context.ParametersObject is TimeWindowFilterSettings))
73+
if (context.Settings != null && !(context.Settings is TimeWindowFilterSettings))
7874
{
7975
throw new ArgumentException(
80-
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(TimeWindowFilterSettings)}'.",
81-
nameof(context.ParametersObject));
76+
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.Settings)} value of type '{context.Settings.GetType()}', but expected '{typeof(TimeWindowFilterSettings)}'.",
77+
nameof(context.Settings));
8278
}
8379

84-
settings = (TimeWindowFilterSettings)context.ParametersObject ?? (TimeWindowFilterSettings)context.Settings ?? (TimeWindowFilterSettings)BindParameters(context.Parameters);
80+
//
81+
// Check if prebound settings available, otherwise bind from parameters.
82+
TimeWindowFilterSettings settings = (TimeWindowFilterSettings)context.Settings ?? (TimeWindowFilterSettings)BindParameters(context.Parameters);
8583

8684
DateTimeOffset now = SystemClock?.GetUtcNow() ?? DateTimeOffset.UtcNow;
8785

‎src/Microsoft.FeatureManagement/FeatureManager.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,11 +498,14 @@ private async ValueTask<bool> IsEnabledAsync<TContext>(FeatureDefinition feature
498498
{
499499
FeatureName = featureDefinition.Name,
500500
Parameters = featureFilterConfiguration.Parameters,
501-
ParametersObject = featureFilterConfiguration.ParametersObject,
501+
Settings = featureFilterConfiguration.ParametersObject,
502502
CancellationToken = cancellationToken
503503
};
504504

505-
BindSettings(filter, context, filterIndex);
505+
if (context.Settings == null)
506+
{
507+
BindSettings(filter, context, filterIndex);
508+
}
506509

507510
//
508511
// IContextualFeatureFilter
@@ -681,6 +684,12 @@ private void BindSettings(IFeatureFilterMetadata filter, FeatureFilterEvaluation
681684
return;
682685
}
683686

687+
// Skip parameter binding if the provider has already supplied a parameters object.
688+
if (context.Settings != null)
689+
{
690+
return;
691+
}
692+
684693
if (!(context.Parameters is ConfigurationWrapper) || Cache == null)
685694
{
686695
context.Settings = binder.BindParameters(context.Parameters);

‎src/Microsoft.FeatureManagement/Targeting/ContextualTargetingFilter.cs‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,16 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, ITargeti
6767
throw new ArgumentNullException(nameof(targetingContext));
6868
}
6969

70-
//
71-
// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
72-
TargetingFilterSettings settings;
73-
74-
if (context.ParametersObject != null && !(context.ParametersObject is TargetingFilterSettings))
70+
if (context.Settings != null && !(context.Settings is TargetingFilterSettings))
7571
{
7672
throw new ArgumentException(
77-
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(TargetingFilterSettings)}'.",
78-
nameof(context.ParametersObject));
73+
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.Settings)} value of type '{context.Settings.GetType()}', but expected '{typeof(TargetingFilterSettings)}'.",
74+
nameof(context.Settings));
7975
}
8076

81-
settings = (TargetingFilterSettings)context.ParametersObject ?? (TargetingFilterSettings)context.Settings ?? (TargetingFilterSettings)BindParameters(context.Parameters);
77+
//
78+
// Check if prebound settings available, otherwise bind from parameters.
79+
TargetingFilterSettings settings = (TargetingFilterSettings)context.Settings ?? (TargetingFilterSettings)BindParameters(context.Parameters);
8280

8381
return Task.FromResult(TargetingEvaluator.IsTargeted(targetingContext, settings, _options.IgnoreCase, context.FeatureName));
8482
}

‎tests/Tests.FeatureManagement/FeatureManagementTest.cs‎

Lines changed: 118 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,9 +1094,9 @@ public async Task UsesParametersObject()
10941094
testFeatureFilter.Callback = (evaluationContext) =>
10951095
{
10961096
//
1097-
// When ParametersObject is set, it should be available on the context
1098-
// so custom filters can use it with their own precedence logic.
1099-
Assert.Same(parameterObject, evaluationContext.ParametersObject);
1097+
// When ParametersObject is set, it should be available on the context via settings
1098+
// so custom filters can use it.
1099+
Assert.Same(parameterObject, evaluationContext.Settings);
11001100

11011101
return Task.FromResult(true);
11021102
};
@@ -1157,7 +1157,6 @@ public async Task ParametersObjectFallsBackToParametersWhenNull()
11571157
//
11581158
// When ParametersObject is null, Settings should be populated
11591159
// by IFilterParametersBinder as usual.
1160-
Assert.Null(evaluationContext.ParametersObject);
11611160
Assert.NotNull(evaluationContext.Settings);
11621161

11631162
return Task.FromResult(true);
@@ -1925,6 +1924,121 @@ public async Task TimeWindowFilterThrowsOnInvalidParametersObjectType()
19251924

19261925
await Assert.ThrowsAsync<ArgumentException>(() => featureManager.IsEnabledAsync("BadFeature"));
19271926
}
1927+
1928+
[Fact]
1929+
public async Task TargetingFilterUsesParametersObject()
1930+
{
1931+
var services = new ServiceCollection();
1932+
1933+
var definitionProvider = new InMemoryFeatureDefinitionProvider(
1934+
new FeatureDefinition[]
1935+
{
1936+
new FeatureDefinition
1937+
{
1938+
Name = "TargetingFeature",
1939+
EnabledFor = new List<FeatureFilterConfiguration>()
1940+
{
1941+
new FeatureFilterConfiguration
1942+
{
1943+
Name = "Microsoft.Targeting",
1944+
ParametersObject = new TargetingFilterSettings
1945+
{
1946+
Audience = new Audience
1947+
{
1948+
Users = new List<string> { "Jeff" },
1949+
Groups = new List<GroupRollout>
1950+
{
1951+
new GroupRollout
1952+
{
1953+
Name = "Ring1",
1954+
RolloutPercentage = 100
1955+
}
1956+
},
1957+
DefaultRolloutPercentage = 0
1958+
}
1959+
}
1960+
}
1961+
}
1962+
}
1963+
});
1964+
1965+
services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
1966+
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
1967+
.AddFeatureManagement();
1968+
1969+
ServiceProvider serviceProvider = services.BuildServiceProvider();
1970+
1971+
IVariantFeatureManager featureManager = serviceProvider.GetRequiredService<IVariantFeatureManager>();
1972+
1973+
//
1974+
// Targeted user should be enabled
1975+
var targetingContext = new TargetingContext
1976+
{
1977+
UserId = "Jeff",
1978+
Groups = new List<string> { "Ring0" }
1979+
};
1980+
1981+
Assert.True(await featureManager.IsEnabledAsync("TargetingFeature", targetingContext));
1982+
1983+
//
1984+
// User in targeted group should be enabled
1985+
targetingContext = new TargetingContext
1986+
{
1987+
UserId = "NotTargeted",
1988+
Groups = new List<string> { "Ring1" }
1989+
};
1990+
1991+
Assert.True(await featureManager.IsEnabledAsync("TargetingFeature", targetingContext));
1992+
1993+
//
1994+
// Non-targeted user should be disabled (0% default rollout)
1995+
targetingContext = new TargetingContext
1996+
{
1997+
UserId = "NotTargeted",
1998+
Groups = new List<string> { "Ring0" }
1999+
};
2000+
2001+
Assert.False(await featureManager.IsEnabledAsync("TargetingFeature", targetingContext));
2002+
}
2003+
2004+
[Fact]
2005+
public async Task TargetingFilterThrowsOnInvalidParametersObjectType()
2006+
{
2007+
var services = new ServiceCollection();
2008+
2009+
var definitionProvider = new InMemoryFeatureDefinitionProvider(
2010+
new FeatureDefinition[]
2011+
{
2012+
new FeatureDefinition
2013+
{
2014+
Name = "BadFeature",
2015+
EnabledFor = new List<FeatureFilterConfiguration>()
2016+
{
2017+
new FeatureFilterConfiguration
2018+
{
2019+
Name = "Microsoft.Targeting",
2020+
ParametersObject = "wrong type"
2021+
}
2022+
}
2023+
}
2024+
});
2025+
2026+
services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
2027+
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
2028+
.AddFeatureManagement();
2029+
2030+
ServiceProvider serviceProvider = services.BuildServiceProvider();
2031+
2032+
IVariantFeatureManager featureManager = serviceProvider.GetRequiredService<IVariantFeatureManager>();
2033+
2034+
var targetingContext = new TargetingContext
2035+
{
2036+
UserId = "Jeff",
2037+
Groups = new List<string>()
2038+
};
2039+
2040+
await Assert.ThrowsAsync<ArgumentException>(() => featureManager.IsEnabledAsync("BadFeature", targetingContext).AsTask());
2041+
}
19282042
}
19292043

19302044
public class FeatureManagementVariantTest

0 commit comments

Comments
 (0)