Support contextual variant service provider and feature status fallback - #611
Open
Zhiyuan Liang (zhiyuanliang-ms) wants to merge 2 commits into
Open
Support contextual variant service provider and feature status fallback#611Zhiyuan Liang (zhiyuanliang-ms) wants to merge 2 commits into
Zhiyuan Liang (zhiyuanliang-ms) wants to merge 2 commits into
Conversation
Copilot started reviewing on behalf of
Zhiyuan Liang (zhiyuanliang-ms)
July 29, 2026 09:47
View session
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR expands the variant-service injection capabilities in the Feature Management .NET SDK by introducing a contextual variant service provider API and adding an optional “feature enabled/disabled” fallback path when no variant-based service can be resolved.
Changes:
- Introduces
IContextualVariantServiceProvider<TService>and updates the internalVariantServiceProvider<TService>to support context-aware evaluation. - Adds
WithVariantService<TService, TEnabled, TDisabled>(featureName)to allow falling back to an enabled/disabled implementation when variant resolution fails. - Refactors and extends tests into a dedicated
VariantServiceProviderTestsuite, including new coverage for contextual behavior and feature-status fallback.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Tests.FeatureManagement/VariantServices.cs | Adds alias-based test implementations used by new variant/fallback tests. |
| tests/Tests.FeatureManagement/VariantServiceProviderTest.cs | New test suite covering variant DI, keyed service resolution, contextual provider behavior, and status fallback. |
| tests/Tests.FeatureManagement/FeatureManagementTest.cs | Removes variant service provider tests that were moved to the new dedicated test file. |
| tests/Tests.FeatureManagement/AppContext.cs | Adds an additional context type for tests (currently unused). |
| src/Microsoft.FeatureManagement/VariantServiceProvider.cs | Implements contextual service retrieval and optional feature-status fallback. |
| src/Microsoft.FeatureManagement/IContextualVariantServiceProvider.cs | New public interface for contextual variant service providers. |
| src/Microsoft.FeatureManagement/FeatureManagementBuilderExtensions.cs | Adds new DI builder overload enabling feature-status fallback and registers contextual provider interface. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
linglingye001
approved these changes
Aug 11, 2026
Member
Author
|
Hey Степан (@Stepami), do you have any concern about this PR? |
|
hey Zhiyuan Liang (@zhiyuanliang-ms)! i'll take a look today, been busy for the last few weeks |
Comment on lines
+119
to
+126
| Type implementationType = enabled ? _fallbackWhenEnabled : _fallbackWhenDisabled; | ||
|
|
||
| if (implementationType != null) | ||
| { | ||
| return _variantServiceCache.GetOrAdd(GetVariantServiceName(implementationType), ResolveVariantService); | ||
| } | ||
|
|
||
| return null; |
There was a problem hiding this comment.
so if i want to obtain service based on status from keyed do i need to register it by string name?
services.AddSingleton<IService, EnabledService>(nameof(EnabledService));i would like to update docs in this PR
Comment on lines
+110
to
117
| if (useContext) | ||
| { | ||
| enabled = await _featureManager.IsEnabledAsync(_featureName, context, cancellationToken); | ||
| } | ||
| else | ||
| { | ||
| enabled = await _featureManager.IsEnabledAsync(_featureName, cancellationToken); | ||
| } |
There was a problem hiding this comment.
do we really need this branching here?
await _featureManager.IsEnabledAsync(_featureName, cancellationToken)is basically
await _featureManager.IsEnabledAsync<object>(_featureName, null, cancellationToken)useContext is internal configuration, so a developer can't change it
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR?
#604 #608
IVariantServiceProvider<TService>.GetServiceAsynccurrently does not accept a context, which limits it to scenarios that rely on ambient context.This creates a capability gap between
VariantServiceProviderandIVariantFeatureManager, because bothIVariantFeatureManager.IsEnabledAsyncandIVariantFeatureManager.GetVariantAsyncprovide overloads that accept an explicit context.How to fix
This PR introduces
IContextualVariantServiceProvider<TService>with methodGetServiceAsync<TContext>(TContext context, CancellationToken cancellationToken)The PR also adds
WithVariantService<TService, TEnabled, TDisabled>(featureName)which supports falling back to a service based on the feature’s enabled or disabled status.Service resolution behavior
Service resolution follows these steps:
GetVariantAsync.GetVariantAsynconly acceptsITargetingContext, so a non-targeting context is not used during variant resolution.TEnabled.TDisabled.Because
IsEnabledAsyncaccepts an arbitraryTContext, the feature-status fallback honors the supplied context even when it does not implementITargetingContext.