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

Resolves snapshot breaking change and adjusts feature tag helper as well #505

Merged
merged 3 commits into from
Sep 26, 2024
Merged

Resolves snapshot breaking change and adjusts feature tag helper as well #505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8be45cd
Resolves snapshot breaking change and adjusts feature tag helper as well
rossgrambo Sep 25, 2024
ea8fea1
Moves comment out of summary
rossgrambo Sep 25, 2024
3e07522
Adjusted snapshot to use a shared IsEnabled cache
rossgrambo Sep 25, 2024
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
10 changes: 7 additions & 3 deletions src/Microsoft.FeatureManagement.AspNetCore/TagHelpers/FeatureTagHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ namespace Microsoft.FeatureManagement.Mvc.TagHelpers
/// </summary>
public class FeatureTagHelper : TagHelper
{
private readonly IVariantFeatureManager _featureManager;
private readonly IFeatureManager _featureManager;
private readonly IVariantFeatureManager _variantFeatureManager;

/// <summary>
/// A feature name, or comma separated list of feature names, for which the content should be rendered. By default, all specified features must be enabled to render the content, but this requirement can be controlled by the <see cref="Requirement"/> property.
Expand All @@ -41,9 +42,12 @@ public class FeatureTagHelper : TagHelper
/// Creates a feature tag helper.
/// </summary>
/// <param name="featureManager">The feature manager snapshot to use to evaluate feature state.</param>
public FeatureTagHelper(IVariantFeatureManagerSnapshot featureManager)
/// <param name="variantFeatureManager">The variant feature manager snapshot to use to evaluate feature state.</param>
public FeatureTagHelper(IFeatureManagerSnapshot featureManager, IVariantFeatureManagerSnapshot variantFeatureManager)
{
// Takes both a feature manager and a variant feature manager for backwards compatibility.
_featureManager = featureManager ?? throw new ArgumentNullException(nameof(featureManager));
_variantFeatureManager = variantFeatureManager ?? throw new ArgumentNullException(nameof(variantFeatureManager));
}

/// <summary>
Expand Down Expand Up @@ -84,7 +88,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu
enabled = await variants.Any(
async variant =>
{
Variant assignedVariant = await _featureManager.GetVariantAsync(features.First()).ConfigureAwait(false);
Variant assignedVariant = await _variantFeatureManager.GetVariantAsync(features.First()).ConfigureAwait(false);

return variant == assignedVariant?.Name;
});
Expand Down
40 changes: 29 additions & 11 deletions src/Microsoft.FeatureManagement/FeatureManagerSnapshot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,37 @@ namespace Microsoft.FeatureManagement
/// </summary>
class FeatureManagerSnapshot : IFeatureManagerSnapshot, IVariantFeatureManagerSnapshot
{
private readonly IVariantFeatureManager _featureManager;
private readonly IFeatureManager _featureManager;
private readonly IVariantFeatureManager _variantFeatureManager;
private readonly ConcurrentDictionary<string, ValueTask<bool>> _flagCache = new ConcurrentDictionary<string, ValueTask<bool>>();
private readonly ConcurrentDictionary<string, Variant> _variantCache = new ConcurrentDictionary<string, Variant>();
private IEnumerable<string> _featureNames;

public FeatureManagerSnapshot(IVariantFeatureManager featureManager)
// Takes both a feature manager and a variant feature manager for backwards compatibility.
public FeatureManagerSnapshot(IFeatureManager featureManager, IVariantFeatureManager variantFeatureManager)
{
_featureManager = featureManager ?? throw new ArgumentNullException(nameof(featureManager));
_variantFeatureManager = variantFeatureManager ?? throw new ArgumentNullException(nameof(variantFeatureManager));
}

public IAsyncEnumerable<string> GetFeatureNamesAsync()
public async IAsyncEnumerable<string> GetFeatureNamesAsync()
{
return GetFeatureNamesAsync(CancellationToken.None);
if (_featureNames == null)
{
var featureNames = new List<string>();

await foreach (string featureName in _featureManager.GetFeatureNamesAsync().ConfigureAwait(false))
{
featureNames.Add(featureName);
}

_featureNames = featureNames;
}

foreach (string featureName in _featureNames)
{
yield return featureName;
}
}

public async IAsyncEnumerable<string> GetFeatureNamesAsync([EnumeratorCancellation] CancellationToken cancellationToken)
Expand All @@ -37,7 +55,7 @@ public async IAsyncEnumerable<string> GetFeatureNamesAsync([EnumeratorCancellati
{
var featureNames = new List<string>();

await foreach (string featureName in _featureManager.GetFeatureNamesAsync(cancellationToken).ConfigureAwait(false))
await foreach (string featureName in _variantFeatureManager.GetFeatureNamesAsync(cancellationToken).ConfigureAwait(false))
{
featureNames.Add(featureName);
}
Expand All @@ -55,28 +73,28 @@ public Task<bool> IsEnabledAsync(string feature)
{
return _flagCache.GetOrAdd(
feature,
(key) => _featureManager.IsEnabledAsync(key, CancellationToken.None)).AsTask();
(key) => new ValueTask<bool>(_featureManager.IsEnabledAsync(key))).AsTask();
}

public Task<bool> IsEnabledAsync<TContext>(string feature, TContext context)
{
return _flagCache.GetOrAdd(
feature,
(key) => _featureManager.IsEnabledAsync(key, context, CancellationToken.None)).AsTask();
(key) => new ValueTask<bool>(_featureManager.IsEnabledAsync(key, context))).AsTask();
}

public ValueTask<bool> IsEnabledAsync(string feature, CancellationToken cancellationToken)
{
return _flagCache.GetOrAdd(
feature,
(key) => _featureManager.IsEnabledAsync(key, cancellationToken));
(key) => _variantFeatureManager.IsEnabledAsync(key, cancellationToken));
}

public ValueTask<bool> IsEnabledAsync<TContext>(string feature, TContext context, CancellationToken cancellationToken)
{
return _flagCache.GetOrAdd(
feature,
(key) => _featureManager.IsEnabledAsync(key, context, cancellationToken));
(key) => _variantFeatureManager.IsEnabledAsync(key, context, cancellationToken));
}

public async ValueTask<Variant> GetVariantAsync(string feature, CancellationToken cancellationToken)
Expand All @@ -90,7 +108,7 @@ public async ValueTask<Variant> GetVariantAsync(string feature, CancellationToke
return _variantCache[cacheKey];
}

Variant variant = await _featureManager.GetVariantAsync(feature, cancellationToken).ConfigureAwait(false);
Variant variant = await _variantFeatureManager.GetVariantAsync(feature, cancellationToken).ConfigureAwait(false);

_variantCache[cacheKey] = variant;

Expand All @@ -108,7 +126,7 @@ public async ValueTask<Variant> GetVariantAsync(string feature, ITargetingContex
return _variantCache[cacheKey];
}

Variant variant = await _featureManager.GetVariantAsync(feature, context, cancellationToken).ConfigureAwait(false);
Variant variant = await _variantFeatureManager.GetVariantAsync(feature, context, cancellationToken).ConfigureAwait(false);

_variantCache[cacheKey] = variant;

Expand Down
94 changes: 38 additions & 56 deletions src/Microsoft.FeatureManagement/ServiceCollectionExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec
"Scoped feature management has been registered.");
}

services.AddLogging();

services.AddMemoryCache();

//
// Add required services
services.TryAddSingleton<IFeatureDefinitionProvider, ConfigurationFeatureDefinitionProvider>();
AddCommonFeatureManagementServices(services);

services.AddSingleton(sp => new FeatureManager(
sp.GetRequiredService<IFeatureDefinitionProvider>(),
Expand All @@ -58,27 +52,7 @@ public static IFeatureManagementBuilder AddFeatureManagement(this IServiceCollec

services.TryAddSingleton<IVariantFeatureManager>(sp => sp.GetRequiredService<FeatureManager>());

services.AddScoped<FeatureManagerSnapshot>();

services.TryAddScoped<IFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());

services.TryAddScoped<IVariantFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());

var builder = new FeatureManagementBuilder(services);

//
// Add built-in feature filters
builder.AddFeatureFilter<PercentageFilter>();

builder.AddFeatureFilter<TimeWindowFilter>(sp =>
new TimeWindowFilter()
{
Cache = sp.GetRequiredService<IMemoryCache>()
});

builder.AddFeatureFilter<ContextualTargetingFilter>();

return builder;
return GetFeatureManagementBuilder(services);
}

/// <summary>
Expand Down Expand Up @@ -120,13 +94,7 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService
"Singleton feature management has been registered.");
}

services.AddLogging();

services.AddMemoryCache();

//
// Add required services
services.TryAddSingleton<IFeatureDefinitionProvider, ConfigurationFeatureDefinitionProvider>();
AddCommonFeatureManagementServices(services);

services.AddScoped(sp => new FeatureManager(
sp.GetRequiredService<IFeatureDefinitionProvider>(),
Expand All @@ -144,27 +112,7 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService

services.TryAddScoped<IVariantFeatureManager>(sp => sp.GetRequiredService<FeatureManager>());

services.AddScoped<FeatureManagerSnapshot>();

services.TryAddScoped<IFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());

services.TryAddScoped<IVariantFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());

var builder = new FeatureManagementBuilder(services);

//
// Add built-in feature filters
builder.AddFeatureFilter<PercentageFilter>();

builder.AddFeatureFilter<TimeWindowFilter>(sp =>
new TimeWindowFilter()
{
Cache = sp.GetRequiredService<IMemoryCache>()
});

builder.AddFeatureFilter<ContextualTargetingFilter>();

return builder;
return GetFeatureManagementBuilder(services);
}

/// <summary>
Expand All @@ -190,5 +138,39 @@ public static IFeatureManagementBuilder AddScopedFeatureManagement(this IService

return services.AddScopedFeatureManagement();
}

private static void AddCommonFeatureManagementServices(IServiceCollection services)
{
services.AddLogging();

services.AddMemoryCache();

services.TryAddSingleton<IFeatureDefinitionProvider, ConfigurationFeatureDefinitionProvider>();

services.AddScoped<FeatureManagerSnapshot>();

services.TryAddScoped<IFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());

services.TryAddScoped<IVariantFeatureManagerSnapshot>(sp => sp.GetRequiredService<FeatureManagerSnapshot>());
}

private static IFeatureManagementBuilder GetFeatureManagementBuilder(IServiceCollection services)
{
var builder = new FeatureManagementBuilder(services);

//
// Add built-in feature filters
builder.AddFeatureFilter<PercentageFilter>();

builder.AddFeatureFilter<TimeWindowFilter>(sp =>
new TimeWindowFilter()
{
Cache = sp.GetRequiredService<IMemoryCache>()
});

builder.AddFeatureFilter<ContextualTargetingFilter>();

return builder;
}
}
}
69 changes: 69 additions & 0 deletions tests/Tests.FeatureManagement.AspNetCore/FeatureManagementAspNetCore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Razor.TagHelpers;
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.FeatureManagement;
using Microsoft.FeatureManagement.Mvc.TagHelpers;
using System.Collections.Generic;
using System.Linq;
using System.Net;
Expand Down Expand Up @@ -181,4 +183,71 @@ private static void DisableEndpointRouting(MvcOptions options)
options.EnableEndpointRouting = false;
}
}

public class CustomImplementationsFeatureManagementTests
{
public class CustomIFeatureManager : IFeatureManager
{
public IAsyncEnumerable<string> GetFeatureNamesAsync()
{
return new string[1] { "Test" }.ToAsyncEnumerable();
}

public async Task<bool> IsEnabledAsync(string feature)
{
return await Task.FromResult(feature == "Test");
}

public async Task<bool> IsEnabledAsync<TContext>(string feature, TContext context)
{
return await Task.FromResult(feature == "Test");
}
}

[Fact]
public async Task CustomIFeatureManagerAspNetCoreTest()
{
IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();

var services = new ServiceCollection();

services.AddSingleton(config)
.AddSingleton<IFeatureManager, CustomIFeatureManager>()
.AddFeatureManagement(); // Shouldn't override

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

Assert.True(await featureManager.IsEnabledAsync("Test"));
Assert.False(await featureManager.IsEnabledAsync("NotTest"));

// FeatureTagHelper should use available IFeatureManager
FeatureTagHelper featureTagHelper = new FeatureTagHelper(serviceProvider.GetRequiredService<IFeatureManagerSnapshot>(), serviceProvider.GetRequiredService<IVariantFeatureManagerSnapshot>());
TagHelperOutput tagHelperOutput = new TagHelperOutput("TestTag", new TagHelperAttributeList(), (aBool, aHtmlEncoder) => { return null; });

// Test returns true, so it shouldn't be modified
featureTagHelper.Name = "Test";
Assert.False(tagHelperOutput.IsContentModified);
await featureTagHelper.ProcessAsync(null, tagHelperOutput);
Assert.False(tagHelperOutput.IsContentModified);

tagHelperOutput.Reinitialize("TestTag", TagMode.StartTagAndEndTag);

// NotTest returns false, so it should be modified
featureTagHelper.Name = "NotTest";
Assert.False(tagHelperOutput.IsContentModified);
await featureTagHelper.ProcessAsync(null, tagHelperOutput);
Assert.True(tagHelperOutput.IsContentModified);

tagHelperOutput.Reinitialize("TestTag", TagMode.StartTagAndEndTag);

// When variant is used, Test flag should no longer exist and return false
featureTagHelper.Name = "Test";
featureTagHelper.Variant = "Something";
Assert.False(tagHelperOutput.IsContentModified);
await featureTagHelper.ProcessAsync(null, tagHelperOutput);
Assert.True(tagHelperOutput.IsContentModified);
}
}
}
1 change: 1 addition & 0 deletions tests/Tests.FeatureManagement.AspNetCore/Tests.FeatureManagement.AspNetCore.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.0" />
<PackageReference Include="System.Linq.Async" Version="6.0.1" />
<PackageReference Include="xunit" Version="2.9.0" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.8.2" />
</ItemGroup>
Expand Down
Loading
Loading