Skip to content

Commit d434317

Browse files
zhiyuanliang-mszhiyuanliang-ms
committed
1. add UT 2. fix the bug: forget to check the name for contextual filter
1 parent 4586dce commit d434317

File tree

4 files changed

+199
-7
lines changed

4 files changed

+199
-7
lines changed

‎src/Microsoft.FeatureManagement/FeatureManager.cs‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,9 @@ private async Task<bool> IsEnabledAsync<TContext>(string feature, TContext appCo
145145

146146
if (filter == null)
147147
{
148-
string errorMessage = $"The feature filter '{featureFilterConfiguration.Name}' specified for feature '{feature}' was not found.";
148+
string errorMessage = appContext == null
149+
? $"The feature filter '{featureFilterConfiguration.Name}' specified for feature '{feature}' was not found."
150+
: $"The contextual feature filter '{featureFilterConfiguration.Name}' with context type '{appContext.GetType()}', specified for feature '{feature}' was not found.";
149151

150152
if (!_options.IgnoreMissingFeatureFilters)
151153
{
@@ -310,16 +312,16 @@ private IFeatureFilterMetadata GetFeatureFilterMetadata(string filterName, Type
310312

311313
if (appContextType == null)
312314
{
313-
return IsFilterNameMatched(filterType, filterName) && (f is IFeatureFilter);
315+
return (f is IFeatureFilter) && IsFilterNameMatched(filterType, filterName);
314316
}
315317
else
316318
{
317319
IEnumerable<Type> contextualFilterInterfaces = filterType.GetInterfaces().Where(
318-
i => i.IsGenericType
319-
&& i.GetGenericTypeDefinition().IsAssignableFrom(typeof(IContextualFeatureFilter<>))
320-
&& i.GetGenericArguments()[0].IsAssignableFrom(appContextType));
320+
i => i.IsGenericType &&
321+
i.GetGenericTypeDefinition().IsAssignableFrom(typeof(IContextualFeatureFilter<>)) &&
322+
i.GetGenericArguments()[0].IsAssignableFrom(appContextType));
321323

322-
return contextualFilterInterfaces.Any();
324+
return contextualFilterInterfaces.Any() && IsFilterNameMatched(filterType, filterName);
323325
}
324326
});
325327

@@ -354,6 +356,7 @@ private ContextualFeatureFilterEvaluator GetContextualFeatureFilter(string filte
354356
(_) => {
355357

356358
IFeatureFilterMetadata metadata = GetFeatureFilterMetadata(filterName, appContextType);
359+
357360
return new ContextualFeatureFilterEvaluator(metadata, appContextType);
358361
}
359362
);

‎tests/Tests.FeatureManagement/FeatureManagement.cs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public class FeatureManagement
2626
private const string OffFeature = "OffFeature";
2727
private const string ConditionalFeature = "ConditionalFeature";
2828
private const string ContextualFeature = "ContextualFeature";
29+
private const string FeatureUsesFiltersWithDuplicatedAlias = "FeatureUsesFiltersWithDuplicatedAlias";
2930

3031
[Fact]
3132
public async Task ReadsConfiguration()
@@ -121,6 +122,115 @@ public async Task Integrates()
121122
Assert.False(res.Headers.Contains(nameof(RouterMiddleware)));
122123
}
123124

125+
[Fact]
126+
public async Task AllowDuplicatedFilterAlias()
127+
{
128+
const string duplicatedFilterName = "DuplicatedFilterName";
129+
130+
IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();
131+
132+
var services = new ServiceCollection();
133+
134+
services
135+
.AddSingleton(config)
136+
.AddFeatureManagement()
137+
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
138+
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithAccountContext>()
139+
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>();
140+
141+
ServiceProvider serviceProvider = services.BuildServiceProvider();
142+
143+
IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
144+
145+
AppContext appContext = new AppContext();
146+
147+
DummyContext dummyContext = new DummyContext();
148+
149+
Assert.True(await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias));
150+
151+
Assert.True(await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias, appContext));
152+
153+
Assert.True(await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias, dummyContext));
154+
155+
services = new ServiceCollection();
156+
157+
services
158+
.AddSingleton(config)
159+
.AddFeatureManagement()
160+
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>()
161+
.AddFeatureFilter<DuplicatedAliasFeatureFilter2>();
162+
163+
serviceProvider = services.BuildServiceProvider();
164+
165+
featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
166+
167+
FeatureManagementException ex = await Assert.ThrowsAsync<FeatureManagementException>(
168+
async () =>
169+
{
170+
await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias);
171+
});
172+
173+
Assert.Equal($"Multiple feature filters match the configured filter named '{duplicatedFilterName}'.", ex.Message);
174+
175+
services = new ServiceCollection();
176+
177+
services
178+
.AddSingleton(config)
179+
.AddFeatureManagement()
180+
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>()
181+
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext2>();
182+
183+
serviceProvider = services.BuildServiceProvider();
184+
185+
featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
186+
187+
ex = await Assert.ThrowsAsync<FeatureManagementException>(
188+
async () =>
189+
{
190+
await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias, dummyContext);
191+
});
192+
193+
Assert.Equal($"Multiple contextual feature filters match the configured filter named '{duplicatedFilterName}' and context type '{dummyContext.GetType()}'.", ex.Message);
194+
195+
services = new ServiceCollection();
196+
197+
services
198+
.AddSingleton(config)
199+
.AddFeatureManagement()
200+
.AddFeatureFilter<ContextualDuplicatedAliasFeatureFilterWithDummyContext1>();
201+
202+
serviceProvider = services.BuildServiceProvider();
203+
204+
featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
205+
206+
ex = await Assert.ThrowsAsync<FeatureManagementException>(
207+
async () =>
208+
{
209+
await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias);
210+
});
211+
212+
Assert.Equal($"The feature filter '{duplicatedFilterName}' specified for feature '{FeatureUsesFiltersWithDuplicatedAlias}' was not found.", ex.Message);
213+
214+
services = new ServiceCollection();
215+
216+
services
217+
.AddSingleton(config)
218+
.AddFeatureManagement()
219+
.AddFeatureFilter<DuplicatedAliasFeatureFilter1>();
220+
221+
serviceProvider = services.BuildServiceProvider();
222+
223+
featureManager = serviceProvider.GetRequiredService<IFeatureManager>();
224+
225+
ex = await Assert.ThrowsAsync<FeatureManagementException>(
226+
async () =>
227+
{
228+
await featureManager.IsEnabledAsync(FeatureUsesFiltersWithDuplicatedAlias, dummyContext);
229+
});
230+
231+
Assert.Equal($"The contextual feature filter '{duplicatedFilterName}' with context type '{dummyContext.GetType()}', specified for feature '{FeatureUsesFiltersWithDuplicatedAlias}' was not found.", ex.Message);
232+
}
233+
124234
[Fact]
125235
public async Task GatesFeatures()
126236
{

‎tests/Tests.FeatureManagement/FiltersWithDuplicatedAlias.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
//
4+
using Microsoft.FeatureManagement;
5+
using System.Threading.Tasks;
6+
7+
namespace Tests.FeatureManagement
8+
{
9+
interface IDummyContext
10+
{
11+
string DummyProperty { get; }
12+
}
13+
14+
class DummyContext : IDummyContext
15+
{
16+
public string DummyProperty { get; set; }
17+
}
18+
19+
[FilterAlias(Alias)]
20+
class DuplicatedAliasFeatureFilter1 : IFeatureFilter
21+
{
22+
private const string Alias = "DuplicatedFilterName";
23+
24+
public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
25+
{
26+
return Task.FromResult(true);
27+
}
28+
}
29+
30+
[FilterAlias(Alias)]
31+
class DuplicatedAliasFeatureFilter2 : IFeatureFilter
32+
{
33+
private const string Alias = "DuplicatedFilterName";
34+
35+
public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context)
36+
{
37+
return Task.FromResult(true);
38+
}
39+
}
40+
41+
[FilterAlias(Alias)]
42+
class ContextualDuplicatedAliasFeatureFilterWithAccountContext : IContextualFeatureFilter<IAccountContext>
43+
{
44+
private const string Alias = "DuplicatedFilterName";
45+
46+
public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IAccountContext accountContext)
47+
{
48+
return Task.FromResult(true);
49+
}
50+
}
51+
52+
[FilterAlias(Alias)]
53+
class ContextualDuplicatedAliasFeatureFilterWithDummyContext1 : IContextualFeatureFilter<IDummyContext>
54+
{
55+
private const string Alias = "DuplicatedFilterName";
56+
57+
public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IDummyContext dummyContext)
58+
{
59+
return Task.FromResult(true);
60+
}
61+
}
62+
63+
[FilterAlias(Alias)]
64+
class ContextualDuplicatedAliasFeatureFilterWithDummyContext2 : IContextualFeatureFilter<IDummyContext>
65+
{
66+
private const string Alias = "DuplicatedFilterName";
67+
68+
public Task<bool> EvaluateAsync(FeatureFilterEvaluationContext context, IDummyContext dummyContext)
69+
{
70+
return Task.FromResult(true);
71+
}
72+
}
73+
}

‎tests/Tests.FeatureManagement/appsettings.json‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@
99
"FeatureManagement": {
1010
"OnTestFeature": true,
1111
"OffTestFeature": false,
12+
"FeatureUsesFiltersWithDuplicatedAlias": {
13+
"EnabledFor": [
14+
{
15+
"Name": "DuplicatedFilterName"
16+
}
17+
]
18+
},
1219
"TargetingTestFeature": {
1320
"EnabledFor": [
1421
{
@@ -146,7 +153,6 @@
146153
}
147154
}
148155
]
149-
150156
}
151157
}
152158
}

0 commit comments

Comments
 (0)