Update FilterCollectionExtensions

[zhiyuanliang-ms]

Why this PR?

Update FilterCollectionExtensions #560

Visible change

I really want to change the return type of this API to FilterCollection:

public static IFilterMetadata AddForFeature<TFilterType>(this FilterCollection filters, string feature) where TFilterType : IAsyncActionFilter
{
    IFilterMetadata filterMetadata = null;

    filters.Add(new FeatureGatedAsyncActionFilter<TFilterType>(RequirementType.Any, false, feature));

    return filterMetadata;
}

Because, filterMetadata will always be returned as null. But as @rossgrambo mentioned, this is a breaking change.

New APIs are added:

public static FilterCollection AddForFeature<TFilterType>(this FilterCollection filters, params string[] features)

public static FilterCollection AddForFeature<TFilterType>(this FilterCollection filters, RequirementType requirementType, params string[] features)

public static FilterCollection AddForFeature<TFilterType>(this FilterCollection filters, RequirementType requirementType, bool negate, params string[] features)

Usage:

services.AddMvcCore(o =>
{
    o.Filters.AddForFeature<MvcFilter1>("feature1")
                .AddForFeature<MvcFilter2>(RequirementType.All, "feature1", "feature2");
});

[rossgrambo]

This is a "breaking" change so I'm not sure if we can do it. The reason "breaking" is in quotes is because it looks like we only ever return null here... But in the rare case where someone has:

services.AddMvc(o => 
{
    IFilterMetadata metadata = o.Filters.AddForFeature<ThirdPartyActionFilter>("MyFeatureFlag");
});

We would break them.

[zhiyuanliang-ms]

@jimmyca15 How about make this PR targeting on preview branch to avoid making breaking changes on our current stable release?

[zhiyuanliang-ms]

This is a "breaking" change so I'm not sure if we can do it. The reason "breaking" is in quotes is because it looks like we only ever return null here... But in the rare case where someone has:

services.AddMvc(o => 
{
    IFilterMetadata metadata = o.Filters.AddForFeature<ThirdPartyActionFilter>("MyFeatureFlag");
});

We would break them.

AddForFeature always return null. This is a bug.

[jimmyca15]

Suggested change

	IFilterMetadata filterMetadata = null;
	filters.Add(new FeatureGatedAsyncActionFilter<TFilterType>(feature));
	filters.Add(new FeatureGatedAsyncActionFilter<TFilterType>(RequirementType.Any, false, feature));
	return filterMetadata;
	return filters.Add(new FeatureGatedAsyncActionFilter<TFilterType>(RequirementType.Any, false, feature));

[zhiyuanliang-ms]

The above return type is not IFilterMetadata.

Only FilterCollection.Add<T> returns IFilterMetadata. https://github.com/dotnet/aspnetcore/blob/06a440549690d5dba8e3501c21c61907da69a733/src/Mvc/Mvc.Core/src/Filters/FilterCollection.cs#L12

[jimmyca15]

Discussed offline, AddForFeature should return IFilterMetadata just like FilterCollection.Add does.