Skip to content

Update missing filter handling #590

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

Open
linglingye001 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update missing filter handling #590

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
18 changes: 9 additions & 9 deletions src/Microsoft.FeatureManagement/FeatureManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,15 +418,6 @@ private async ValueTask<bool> IsEnabledAsync<TContext>(FeatureDefinition feature
}
else
{
//
// Ensure no conflicts in the feature definition
if (featureDefinition.RequirementType == RequirementType.All && _options.IgnoreMissingFeatureFilters)
{
throw new FeatureManagementException(
FeatureManagementError.Conflict,
$"The 'IgnoreMissingFeatureFilters' flag cannot be used in combination with a feature of requirement type 'All'.");
}

//
// If the requirement type is all, we default to true. Requirement type All will end on a false
enabled = featureDefinition.RequirementType == RequirementType.All;
Expand Down Expand Up @@ -491,6 +482,15 @@ private async ValueTask<bool> IsEnabledAsync<TContext>(FeatureDefinition feature

Logger?.LogWarning(FeatureFilterNotFoundError, featureFilterConfiguration.Name, featureDefinition.Name);

//
// If requirement type is All, a missing filter means the feature cannot be enabled
if (featureDefinition.RequirementType == RequirementType.All)
{
enabled = false;

break;
}

continue;
}

Expand Down
10 changes: 5 additions & 5 deletions tests/Tests.FeatureManagement/FeatureManagementTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ public async Task UsesRequirementType()
}

[Fact]
public async Task RequirementTypeAllExceptions()
public async Task RequirementTypeAllWithIgnoreMissingFeatureFilters()
{
IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();

Expand All @@ -816,12 +816,12 @@ public async Task RequirementTypeAllExceptions()

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

//
// AllFilterFeature has RequirementType.All with missing filters.
// With IgnoreMissingFeatureFilters enabled, the feature should be treated as disabled.
string allFilterFeature = Features.AllFilterFeature;

await Assert.ThrowsAsync<FeatureManagementException>(async () =>
{
await featureManager.IsEnabledAsync(allFilterFeature);
});
Assert.False(await featureManager.IsEnabledAsync(allFilterFeature));
}

[Fact]
Expand Down
Loading