No possibility to run contextual filters when calling GetVariantAsync

[zhiyuanliang-ms]

The current signature of IVariantFeatureManager.GetVariantAsync is

ValueTask<Variant> GetVariantAsync(string feature, TargetingContext context, CancellationToken cancellationToken = default);

It accepts the parameter of type TargetingContext.

For contextual feature filter:

public interface IContextualFeatureFilter<TContext> : IFeatureFilterMetadata
{
    Task<bool> EvaluateAsync(FeatureFilterEvaluationContext featureFilterContext, TContext appContext);
}

They can use any type of context for evaluation. When calling GetVariantAsync, the contextual feature filter which requires context of types othen than TargetingContext is unuseable.

We need to change the GetVariantAsync to accept parameter of ITargetingContext.

[jimmyca15]

We need to change the GetVariantAsync to accept parameter of ITargetingContext

I was thinking about this and it may not be so straight forward. Using an interface, ITargetingContext limits us from adding properties. Adding properties to an interface is a breaking change. Given I know that we want to do things like add claims based targeting, I think it may be problematic to take the ITargetingContext interface.

Some alternatives I'm considering it to have

interface IVariantFeatureManager
{
...

    ValueTask<Variant> GetVariantAsync(string feature, TContext appContext, CancellationToken cancellationToken = default);
}

and an extension method

ValueTask<Variant> GetVariantAsync(this IVariantFeatureManager, string feature, TargetingContext context, CancellationToken cancellationToken = default);

For syntax sugar/discovery

[zhiyuanliang-ms]

Adding properties to an interface is a breaking change. Given I know that we want to do things like add claims based targeting, I think it may be problematic to take the ITargetingContext interface.

Some alternatives I'm considering it to have

interface IVariantFeatureManager
{
...
    ValueTask<Variant> GetVariantAsync(string feature, TContext appContext, CancellationToken cancellationToken = default);
}

I agree with this.

But if we add claims to TargetingContext in the future, will we also update the ITargetingContext? If yes, we cannot avoid the potential breaking change.

[jimmyca15]

But if we add claims to TargetingContext in the future, will we also update the ITargetingContext? If yes, we cannot avoid the potential breaking change.

To avoid a breaking change we'll have to come up with a new interface like IClaimsTargetingContext.