microsoft / microsoft/FeatureManagement-Dotnet

GetFeatureNames can return duplicates

Open
#534 2 comments 2 reactions 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.