microsoft / microsoft/FeatureManagement-Dotnet
GetFeatureNames can return duplicates
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 1.2k
- Forks
- 129
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 5
Description
In experimenting with transitioning to the MS feat mgmt schema, I added configuration for it under feature_management (alongside existing configuration under FeatureManagement).
In doing so, I noticed that GetFeatureNamesAsync returned the name of that feature multiple times. That actually broke this code we had in one of our apps which surfaces all the flags in one go to an old UI:
var featureNamesEnumerable = _featureManager.GetFeatureNamesAsync();
await featureNamesEnumerable.ForEachAwaitAsync(async featureName =>
{
var isEnabled = await _featureManager.IsEnabledAsync(featureName);
featureFlags.Add(featureName, isEnabled);
});
(note, needs System.Linq.Async nuget package).
Given that IsEnabledAsync digs into the provider and checks for MS Feature Mgmt schema config first, before falling back to the .NET Feature Mgmt schema config after, i.e. it doesn't matter that there's dupe config, it supports this, it feels a bit unexpected to see the feature name appear twice. Easy fix is:
var featureNamesEnumerable = _featureManager.GetFeatureNamesAsync().Distinct();
// ...
But wondering if it's worth doing that in the GetFeatureNamesAsync implementation? Happy to submit a PR if that's approved, as I also need to submit my other PR for the MVC tag helper and can knock both PRs out in one go.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at the GetFeatureNamesAsync implementation and reproduce the duplicate-name result with configuration under both feature_management and FeatureManagement. Confirm that the returned feature names are unique while existing IsEnabledAsync behavior remains unchanged, then add or update coverage if the surrounding implementation has tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100