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.

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

Add IFeatureService & Related Utilities #37

Merged
merged 11 commits into from
Oct 25, 2024
Original file line number Diff line number Diff line change
@@ -1,7 +1,21 @@
using Bitwarden.Extensions.Hosting.Features;

var builder = WebApplication.CreateBuilder(args);

builder.UseBitwardenDefaults();

var app = builder.Build();

app.UseRouting();

app.UseFeatureFlagChecks();

app.MapGet("/", (IConfiguration config) => ((IConfigurationRoot)config).GetDebugView());

app.MapGet("/requires-feature", (IFeatureService featureService) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

๐ŸŒฑ Should this example code be expected to declare known flags?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's declaring its known flags in appsettings.Development.json.

{
return featureService.GetAll();
})
.RequireFeature("feature-one");

app.Run();
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,13 @@
"Default": "Information",
"Microsoft.AspNetCore": "Warning"
}
},
"Features": {
"FlagValues": {
"feature-one": true,
"feature-two": 1,
"feature-three": "my-value"
},
"KnownFlags": ["feature-one", "feature-two"]
}
}
60 changes: 0 additions & 60 deletions extensions/Bitwarden.Extensions.Hosting/src/AssemblyHelpers.cs

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
๏ปฟusing Bitwarden.Extensions.Hosting.Exceptions;
using Bitwarden.Extensions.Hosting.Exceptions;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.Extensions.DependencyInjection;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="LaunchDarkly.ServerSdk" Version="8.5.2" />
<PackageReference Include="Microsoft.Extensions.Hosting" Version="8.0.1" />
<PackageReference Include="OpenTelemetry.Exporter.OpenTelemetryProtocol" Version="1.9.0" />
<PackageReference Include="OpenTelemetry.Extensions.Hosting" Version="1.9.0" />
Expand All @@ -43,5 +44,9 @@
<ItemGroup>
<None Include=".\README.md" Pack="true" PackagePath="\" />
</ItemGroup>


<ItemGroup>
<InternalsVisibleTo Include="Bitwarden.Extensions.Hosting.Tests" />
</ItemGroup>

</Project>
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
๏ปฟnamespace Bitwarden.Extensions.Hosting;
namespace Bitwarden.Extensions.Hosting;

/// <summary>
/// Options for configuring the Bitwarden host.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
using Bitwarden.Extensions.Hosting.Features;

namespace Microsoft.AspNetCore.Builder;

/// <summary>
/// Feature extension methods for <see cref="IApplicationBuilder"/>.
/// </summary>
public static class FeatureApplicationBuilderExtensions
{
/// <summary>
/// Adds the <see cref="FeatureCheckMiddleware"/> to the specified <see cref="IApplicationBuilder"/>, which enabled feature check capabilities.
/// <para>
/// This call must take place between <c>app.UseRouting()</c> and <c>app.UseEndpoints(...)</c> for middleware to function properly.
/// </para>
/// </summary>
/// <param name="app">The <see cref="IApplicationBuilder"/> to add the middleware to.</param>
/// <returns>A reference to <paramref name="app"/> after the operation has completed.</returns>
public static IApplicationBuilder UseFeatureFlagChecks(this IApplicationBuilder app)
{
ArgumentNullException.ThrowIfNull(app);

// This would be a good time to make sure that IFeatureService is registered but it is a scoped service
// and I don't think creating a scope is worth it for that. If we think this is a problem we can add another
// marker interface that is registered as a singleton and validate that it exists here.

app.UseMiddleware<FeatureCheckMiddleware>();
return app;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;

namespace Bitwarden.Extensions.Hosting.Features;

internal sealed class FeatureCheckMiddleware
{
private readonly RequestDelegate _next;
private readonly IHostEnvironment _hostEnvironment;
private readonly IProblemDetailsService _problemDetailsService;
private readonly ILogger<FeatureCheckMiddleware> _logger;

public FeatureCheckMiddleware(
RequestDelegate next,
IHostEnvironment hostEnvironment,
IProblemDetailsService problemDetailsService,
ILogger<FeatureCheckMiddleware> logger)
{
ArgumentNullException.ThrowIfNull(next);
ArgumentNullException.ThrowIfNull(hostEnvironment);
ArgumentNullException.ThrowIfNull(problemDetailsService);
ArgumentNullException.ThrowIfNull(logger);

_next = next;
_hostEnvironment = hostEnvironment;
_problemDetailsService = problemDetailsService;
_logger = logger;
}

public Task Invoke(HttpContext context, IFeatureService featureService)
{
// This middleware is expected to be placed after `UseRouting()` which will fill in this endpoint
var endpoint = context.GetEndpoint();

if (endpoint == null)
{
_logger.LogNoEndpointWarning();
return _next(context);
}

var featureMetadatas = endpoint.Metadata.GetOrderedMetadata<IFeatureMetadata>();

foreach (var featureMetadata in featureMetadatas)
{
if (!featureMetadata.FeatureCheck(featureService))
{
// Do not execute more of the pipeline, return early.
return HandleFailedFeatureCheck(context, featureMetadata);
}

// Continue checking
}

// Either there were no feature checks, or none were failed. Continue on in the pipeline.
return _next(context);
}

private async Task HandleFailedFeatureCheck(HttpContext context, IFeatureMetadata failedFeature)
{
if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogFailedFeatureCheck(failedFeature.ToString()!);
}

Check warning on line 65 in extensions/Bitwarden.Extensions.Hosting/src/Features/FeatureCheckMiddleware.cs

View check run for this annotation

Codecov / codecov/patch

extensions/Bitwarden.Extensions.Hosting/src/Features/FeatureCheckMiddleware.cs#L63-L65

Added lines #L63 - L65 were not covered by tests

context.Response.StatusCode = StatusCodes.Status404NotFound;

var problemDetails = new ProblemDetails();
problemDetails.Title = "Resource not found.";
problemDetails.Status = StatusCodes.Status404NotFound;

// Message added for legacy reasons. We should start preferring title/detail
Copy link
Contributor

Choose a reason for hiding this comment

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

โ“ Legacy? So Message isn't common anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, a lot of things in ASP.NET Core will return a ProblemDetails object unless we specifically go and and change it. So my preference would be to also make that be our return for problems. That way we can start on a unified error format.

It also sends back application/problem+json as the content type so you can introspect on that more on the client side to trust that if you got a bad status code and the content type indicates problem details, you can read the respond body like it's a problem details object and then return a more high quality error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I like that. Do we just get rid of the message then? I'd like to not carry in debt and just expect callers to adjust when they adopt this library.

problemDetails.Extensions["Message"] = "Resource not found.";

// Follow ProblemDetails output type? Would need clients update
if (_hostEnvironment.IsDevelopment())
{
// Add extra information
problemDetails.Detail = $"Feature check failed: {failedFeature}";
}

// We don't really care if this fails, we will return the 404 no matter what.
await _problemDetailsService.TryWriteAsync(new ProblemDetailsContext
{
HttpContext = context,
ProblemDetails = problemDetails,
// TODO: Add metadata?
});
}


}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using Bitwarden.Extensions.Hosting.Features;

namespace Microsoft.AspNetCore.Builder;

/// <summary>
/// Feature extension methods for <see cref="IEndpointConventionBuilder"/>.
/// </summary>
public static class FeatureEndpointConventionBuilderExtensions
{
/// <summary>
/// Adds a feature check for the given feature to the endpoint(s).
/// </summary>
/// <param name="builder">The endpoint convention builder.</param>
/// <param name="featureNameKey"></param>
/// <returns></returns>
public static TBuilder RequireFeature<TBuilder>(this TBuilder builder, string featureNameKey)
where TBuilder : IEndpointConventionBuilder
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(featureNameKey);

builder.Add(endpointBuilder =>
{
endpointBuilder.Metadata.Add(new RequireFeatureAttribute(featureNameKey));
});

return builder;
}

/// <summary>
/// Adds a feature check with the specified check to the endpoint(s).
/// </summary>
/// <param name="builder">The endpoint convention builder.</param>
/// <param name="featureCheck"></param>
/// <returns></returns>
public static TBuilder RequireFeature<TBuilder>(this TBuilder builder, Func<IFeatureService, bool> featureCheck)
where TBuilder : IEndpointConventionBuilder
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(featureCheck);

builder.Add(endpointBuilder =>
{
endpointBuilder.Metadata.Add(new RequireFeatureAttribute(featureCheck));
});

return builder;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
namespace Bitwarden.Extensions.Hosting.Features;

/// <summary>
/// A collection of Launch Darkly specific options.
/// </summary>
public sealed class LaunchDarklyOptions
{
/// <summary>
/// The SdkKey to be used for retrieving feature flag values from Launch Darkly.
/// </summary>
public string? SdkKey { get; set; }
}

/// <summary>
/// A set of options for features.
/// </summary>
public sealed class FeatureFlagOptions
{
/// <summary>
/// All the flags known to this instance, this is used to enumerable values in <see cref="IFeatureService.GetAll()"/>.
/// </summary>
public HashSet<string> KnownFlags { get; set; } = new HashSet<string>();

/// <summary>
/// Flags and flag values to include in the feature flag data source.
/// </summary>
public Dictionary<string, string> FlagValues { get; set; } = new Dictionary<string, string>();

/// <summary>
/// Launch Darkly specific options.
/// </summary>
public LaunchDarklyOptions LaunchDarkly { get; set; } = new LaunchDarklyOptions();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
using Bitwarden.Extensions.Hosting.Features;

namespace Microsoft.Extensions.DependencyInjection;

/// <summary>
/// Extensions for features on <see cref="IServiceCollection"/>.
/// </summary>
public static class ServiceCollectionExtensions
{
/// <summary>
/// Adds known feature flags to the <see cref="FeatureFlagOptions"/>. This makes these flags
/// show up in <see cref="IFeatureService.GetAll()"/>.
/// </summary>
/// <param name="services">The service collection to customize the options on.</param>
/// <param name="knownFlags">The flags to add to the known flags list.</param>
/// <returns>The <see cref="IServiceCollection"/> to chain additional calls.</returns>
public static IServiceCollection AddKnownFeatureFlags(this IServiceCollection services, IEnumerable<string> knownFlags)
{
ArgumentNullException.ThrowIfNull(services);

services.Configure<FeatureFlagOptions>(options =>
{
foreach (var flag in knownFlags)
{
options.KnownFlags.Add(flag);
}
});

return services;
}

/// <summary>
/// Adds feature flags and their values to the <see cref="FeatureFlagOptions"/>.
/// </summary>
/// <param name="services">The service collection to customize the options on.</param>
/// <param name="flagValues">The flags to add to the flag values dictionary.</param>
/// <returns>The <see cref="IServiceCollection"/> to chain additional calls. </returns>
public static IServiceCollection AddFeatureFlagValues(
this IServiceCollection services,
IEnumerable<KeyValuePair<string, string>> flagValues)
{
ArgumentNullException.ThrowIfNull(services);

services.Configure<FeatureFlagOptions>(options =>
{
foreach (var flag in flagValues)
{
options.FlagValues[flag.Key] = flag.Value;
}
});

return services;
}
}
Loading
Loading