Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Register built-in filter by default #287

Merged
merged 10 commits into from
Oct 26, 2023
Merged

Register built-in filter by default #287

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
089b4ce
register built-in filter first & target on main branch
zhiyuanliang-ms Oct 19, 2023
8103728
remove internal keyword
zhiyuanliang-ms Oct 19, 2023
3f1cf32
resolve comments
zhiyuanliang-ms Oct 25, 2023
b529bf7
improve the way to register targeting context accessor
zhiyuanliang-ms Oct 25, 2023
0c781fe
fix typo
zhiyuanliang-ms Oct 25, 2023
eb5bbf3
Merge branch 'main' into zhiyuanliang/register-builtin-filter
zhiyuanliang-ms Oct 25, 2023
94efc1b
Merge branch 'main' into zhiyuanliang/register-builtin-filter
zhiyuanliang-ms Oct 26, 2023
0a7456e
adjust the way to register targeting context accessor & update testcases
zhiyuanliang-ms Oct 26, 2023
e0c12b3
fix typo
zhiyuanliang-ms Oct 26, 2023
c8182b7
move WithTargeting to FeatureManagementBuilderExtension
zhiyuanliang-ms Oct 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/Microsoft.FeatureManagement/FeatureManagementBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.FeatureManagement.FeatureFilters;

namespace Microsoft.FeatureManagement
{
Expand Down Expand Up @@ -37,7 +39,7 @@ public IFeatureManagementBuilder AddFeatureFilter<T>() where T : IFeatureFilterM

if (!Services.Any(descriptor => descriptor.ServiceType == serviceType && descriptor.ImplementationType == implementationType))
{
Services.AddSingleton(typeof(IFeatureFilterMetadata), typeof(T));
Services.AddSingleton(serviceType, implementationType);
}

return this;
Expand All @@ -49,5 +51,14 @@ public IFeatureManagementBuilder AddSessionManager<T>() where T : ISessionManage

return this;
}

public IFeatureManagementBuilder WithTargeting<T>() where T : ITargetingContextAccessor
{
Services.TryAddSingleton(typeof(ITargetingContextAccessor), typeof(T));

AddFeatureFilter<TargetingFilter>();

return this;
}
}
}
8 changes: 8 additions & 0 deletions src/Microsoft.FeatureManagement/IFeatureManagementBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.
//
using Microsoft.Extensions.DependencyInjection;
using Microsoft.FeatureManagement.FeatureFilters;

namespace Microsoft.FeatureManagement
{
Expand Down Expand Up @@ -30,5 +31,12 @@ public interface IFeatureManagementBuilder
/// <typeparam name="T">An implementation of <see cref="ISessionManager"/></typeparam>
/// <returns>The feature management builder.</returns>
IFeatureManagementBuilder AddSessionManager<T>() where T : ISessionManager;

/// <summary>
/// Adds an <see cref="ITargetingContextAccessor"/> to be used for targeting.
/// </summary>
/// <typeparam name="T">An implementation of <see cref="ITargetingContextAccessor"/></typeparam>
/// <returns>The feature management builder.</returns>
IFeatureManagementBuilder WithTargeting<T>() where T : ITargetingContextAccessor;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a method on an interface is a breaking change. This should be an extension method. I believe we already have IFeatureManagementBuilderExtensions

}
}
11 changes: 10 additions & 1 deletion src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;
using Microsoft.FeatureManagement.FeatureFilters;
using System;

namespace Microsoft.FeatureManagement
Expand Down Expand Up @@ -33,7 +34,15 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec

services.AddScoped<IFeatureManagerSnapshot, FeatureManagerSnapshot>();

return new FeatureManagementBuilder(services);
var builder = new FeatureManagementBuilder(services);

builder.AddFeatureFilter<PercentageFilter>();

builder.AddFeatureFilter<TimeWindowFilter>();

builder.AddFeatureFilter<ContextualTargetingFilter>();

return builder;
}

/// <summary>
Expand Down
28 changes: 10 additions & 18 deletions tests/Tests.FeatureManagement/FeatureManagement.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public async Task ReadsOnlyFeatureManagementSection()
}

[Fact]
public async Task AllowDuplicatedFilterAlias()
public async Task AllowsDuplicatedFilterAlias()
{
const string duplicatedFilterName = "DuplicatedFilterName";

Expand All @@ -101,8 +101,7 @@ public async Task AllowDuplicatedFilterAlias()
.AddFeatureManagement()
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>()
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>();

ServiceProvider serviceProvider = services.BuildServiceProvider();

Expand All @@ -127,8 +126,7 @@ public async Task AllowDuplicatedFilterAlias()
services
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>();

serviceProvider = services.BuildServiceProvider();

Expand All @@ -142,8 +140,7 @@ public async Task AllowDuplicatedFilterAlias()
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
.AddFeatureFilter<DuplicatedAliasFeatureFilter2>()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureFilter<DuplicatedAliasFeatureFilter2>();

serviceProvider = services.BuildServiceProvider();

Expand All @@ -163,8 +160,7 @@ public async Task AllowDuplicatedFilterAlias()
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>()
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext2>()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext2>();

serviceProvider = services.BuildServiceProvider();

Expand All @@ -183,8 +179,7 @@ public async Task AllowDuplicatedFilterAlias()
services
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>();

serviceProvider = services.BuildServiceProvider();

Expand Down Expand Up @@ -246,8 +241,7 @@ public async Task TimeWindow()
var serviceCollection = new ServiceCollection();

serviceCollection.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<TimeWindowFilter>();
.AddFeatureManagement();

ServiceProvider provider = serviceCollection.BuildServiceProvider();

Expand All @@ -272,8 +266,7 @@ public async Task Percentage()
var serviceCollection = new ServiceCollection();

serviceCollection.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<PercentageFilter>();
.AddFeatureManagement();

ServiceProvider provider = serviceCollection.BuildServiceProvider();

Expand Down Expand Up @@ -307,8 +300,7 @@ public async Task Targeting()

services
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<ContextualTargetingFilter>();
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

Expand Down Expand Up @@ -374,7 +366,7 @@ public async Task TargetingAccessor()
services
.AddSingleton(config)
.AddFeatureManagement()
.AddFeatureFilter<TargetingFilter>();
.WithTargeting<OnDemandTargetingContextAccessor>();

ServiceProvider serviceProvider = services.BuildServiceProvider();

Expand Down