Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions src/Microsoft.FeatureManagement/FeatureFilterConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,13 @@ public class FeatureFilterConfiguration
/// Configurable parameters that can change across instances of a feature filter.
/// </summary>
public IConfiguration Parameters { get; set; } = new ConfigurationRoot(new List<IConfigurationProvider>());

/// <summary>
/// A parameter object that can be used as an alternative to <see cref="Parameters"/>.
/// Custom <see cref="IFeatureDefinitionProvider"/> implementations can populate this property directly
/// instead of constructing an <see cref="IConfiguration"/> instance.
/// When set, feature filters should prefer this over <see cref="Parameters"/>.
/// </summary>
public object ParametersObject { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ public class FeatureFilterEvaluationContext
/// </summary>
public IConfiguration Parameters { get; set; }

/// <summary>
/// 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.
/// </summary>
public object ParametersObject { get; set; }

/// <summary>
/// A settings object, if any, that has been pre-bound from <see cref="Parameters"/>.
/// The settings are made available for <see cref="IFeatureFilter"/>s that implement <see cref="IFilterParametersBinder"/>.
Expand Down
13 changes: 11 additions & 2 deletions src/Microsoft.FeatureManagement/FeatureFilters/PercentageFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,17 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
}

//
// Check if prebound settings available, otherwise bind from parameters.
PercentageFilterSettings settings = (PercentageFilterSettings)context.Settings ?? (PercentageFilterSettings)BindParameters(context.Parameters);
// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
PercentageFilterSettings settings;

if (context.ParametersObject != null && !(context.ParametersObject is PercentageFilterSettings))
{
throw new ArgumentException(
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(PercentageFilterSettings)}'.",
nameof(context.ParametersObject));
}

settings = (PercentageFilterSettings)context.ParametersObject ?? (PercentageFilterSettings)context.Settings ?? (PercentageFilterSettings)BindParameters(context.Parameters);

bool result = true;

Expand Down
13 changes: 11 additions & 2 deletions src/Microsoft.FeatureManagement/FeatureFilters/TimeWindowFilter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,17 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
}

//
// Check if prebound settings available, otherwise bind from parameters.
TimeWindowFilterSettings settings = (TimeWindowFilterSettings)context.Settings ?? (TimeWindowFilterSettings)BindParameters(context.Parameters);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we set the new parametersObject to context.settings, the code path will be more unified.

// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
TimeWindowFilterSettings settings;

if (context.ParametersObject != null && !(context.ParametersObject is TimeWindowFilterSettings))
{
throw new ArgumentException(
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(TimeWindowFilterSettings)}'.",
nameof(context.ParametersObject));
}

settings = (TimeWindowFilterSettings)context.ParametersObject ?? (TimeWindowFilterSettings)context.Settings ?? (TimeWindowFilterSettings)BindParameters(context.Parameters);

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

Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.FeatureManagement/FeatureManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ private async ValueTask<bool> IsEnabledAsync<TContext>(FeatureDefinition feature
{
FeatureName = featureDefinition.Name,
Parameters = featureFilterConfiguration.Parameters,
ParametersObject = featureFilterConfiguration.ParametersObject,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we set ParametersObject to FeatureFilterEvaluationContext.Settings instead of adding a new ParametersObject property.

@jimmyca15

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea. We'll need an update on the description of Settings. Something like

/// <summary>
/// A settings object, if any, provided for the feature filter to use when evaluating whether the feature should be enabled.
/// This property is populated in two cases:
/// For features that provide parameters as an object.
/// For <see cref="IFeatureFilter"/>s that implement <see cref="IFilterParametersBinder"/>.
/// </summary>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CancellationToken = cancellationToken
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,17 @@ public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, ITargeti
}

//
// Check if prebound settings available, otherwise bind from parameters.
TargetingFilterSettings settings = (TargetingFilterSettings)context.Settings ?? (TargetingFilterSettings)BindParameters(context.Parameters);
// Check if ParametersObject available (takes precedence), then prebound settings, otherwise bind from parameters.
TargetingFilterSettings settings;

if (context.ParametersObject != null && !(context.ParametersObject is TargetingFilterSettings))
{
throw new ArgumentException(
$"The '{Alias}' feature filter for feature '{context.FeatureName}' has a {nameof(context.ParametersObject)} of type '{context.ParametersObject.GetType()}', but expected '{typeof(TargetingFilterSettings)}'.",
nameof(context.ParametersObject));
}

settings = (TargetingFilterSettings)context.ParametersObject ?? (TargetingFilterSettings)context.Settings ?? (TargetingFilterSettings)BindParameters(context.Parameters);

return Task.FromResult(TargetingEvaluator.IsTargeted(targetingContext, settings, _options.IgnoreCase, context.FeatureName));
}
Expand Down
254 changes: 254 additions & 0 deletions tests/Tests.FeatureManagement/FeatureManagementTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,124 @@ public async Task BindsFeatureFlagSettings()

Assert.True(called);
}

[Fact]
public async Task UsesParametersObject()
{
var parameterObject = new object();

FeatureFilterConfiguration testFilterConfiguration = new FeatureFilterConfiguration
{
Name = "Test",
ParametersObject = parameterObject
};

var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = Features.ConditionalFeature,
EnabledFor = new List<FeatureFilterConfiguration>()
{
testFilterConfiguration
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement()
.AddFeatureFilter<TestFilter>();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

IEnumerable<IFeatureFilterMetadata> featureFilters = serviceProvider.GetRequiredService<IEnumerable<IFeatureFilterMetadata>>();

TestFilter testFeatureFilter = (TestFilter)featureFilters.First(f => f is TestFilter);

testFeatureFilter.Callback = (evaluationContext) =>
{
//
// When ParametersObject is set, it should be available on the context
// so custom filters can use it with their own precedence logic.
Assert.Same(parameterObject, evaluationContext.ParametersObject);

return Task.FromResult(true);
};

bool result = await featureManager.IsEnabledAsync(Features.ConditionalFeature);

Assert.True(result);
}

[Fact]
public async Task ParametersObjectFallsBackToParametersWhenNull()
{
FeatureFilterConfiguration testFilterConfiguration = new FeatureFilterConfiguration
{
Name = "Test",
ParametersObject = null
};

var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = Features.ConditionalFeature,
EnabledFor = new List<FeatureFilterConfiguration>()
{
testFilterConfiguration
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement()
.AddFeatureFilter<TestFilter>();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

IEnumerable<IFeatureFilterMetadata> featureFilters = serviceProvider.GetRequiredService<IEnumerable<IFeatureFilterMetadata>>();

TestFilter testFeatureFilter = (TestFilter)featureFilters.First(f => f is TestFilter);

bool binderCalled = false;

testFeatureFilter.ParametersBinderCallback = (parameters) =>
{
binderCalled = true;

return parameters;
};

testFeatureFilter.Callback = (evaluationContext) =>
{
//
// When ParametersObject is null, Settings should be populated
// by IFilterParametersBinder as usual.
Assert.Null(evaluationContext.ParametersObject);
Assert.NotNull(evaluationContext.Settings);

return Task.FromResult(true);
};

bool result = await featureManager.IsEnabledAsync(Features.ConditionalFeature);

Assert.True(result);

Assert.True(binderCalled);
}
}

public class FeatureManagementBuiltInFeatureFilterTest
Expand Down Expand Up @@ -1671,6 +1789,142 @@ public async Task CustomFilterContextualTargetingWithNullSetting()

Assert.True(await featureManager.IsEnabledAsync("CustomFilterFeature"));
}

[Fact]
public async Task PercentageFilterUsesParametersObject()
{
var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = "PercentageFeature",
EnabledFor = new List<FeatureFilterConfiguration>()
{
new FeatureFilterConfiguration
{
Name = "Microsoft.Percentage",
ParametersObject = new PercentageFilterSettings { Value = 100 }
}
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

Assert.True(await featureManager.IsEnabledAsync("PercentageFeature"));
}

[Fact]
public async Task TimeWindowFilterUsesParametersObject()
{
var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = "TimeWindowFeature",
EnabledFor = new List<FeatureFilterConfiguration>()
{
new FeatureFilterConfiguration
{
Name = "Microsoft.TimeWindow",
ParametersObject = new TimeWindowFilterSettings
{
Start = DateTimeOffset.UtcNow.AddDays(-1),
End = DateTimeOffset.UtcNow.AddDays(1)
}
}
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

Assert.True(await featureManager.IsEnabledAsync("TimeWindowFeature"));
}

[Fact]
public async Task PercentageFilterThrowsOnInvalidParametersObjectType()
{
var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = "BadFeature",
EnabledFor = new List<FeatureFilterConfiguration>()
{
new FeatureFilterConfiguration
{
Name = "Microsoft.Percentage",
ParametersObject = "wrong type"
}
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

await Assert.ThrowsAsync<ArgumentException>(() => featureManager.IsEnabledAsync("BadFeature"));
}

[Fact]
public async Task TimeWindowFilterThrowsOnInvalidParametersObjectType()
{
var services = new ServiceCollection();

var definitionProvider = new InMemoryFeatureDefinitionProvider(
new FeatureDefinition[]
{
new FeatureDefinition
{
Name = "BadFeature",
EnabledFor = new List<FeatureFilterConfiguration>()
{
new FeatureFilterConfiguration
{
Name = "Microsoft.TimeWindow",
ParametersObject = 42
}
}
}
});

services.AddSingleton<IFeatureDefinitionProvider>(definitionProvider)
.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build())
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

await Assert.ThrowsAsync<ArgumentException>(() => featureManager.IsEnabledAsync("BadFeature"));
}
}

public class FeatureManagementVariantTest
Expand Down
Loading