-
Notifications
You must be signed in to change notification settings - Fork 66
Use IHostApplicationBuilder for framework application to enable Aspire development #596
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
Conversation
This adds a HttpApplication-derived type that facillitates building with the IHostApplicationBuilder. This allows a lot of the patterns used in modern ASP.NET Core applications to be available in ASP.NET Framework, including simplified hooking up of Aspire components/telemetry/etc.
@CZEMacLeod I'd appreciate any of your thoughts here. It was partially inspired by some of your work - but wanted it minimize any specialized overloads that need to be configured to keep it pretty open |
This all looks pretty good. Will have to try it out and see how it compares to my implementation, and if my scope per request, webapi and mvc dependency injection providers can hook up easily to this. I feel like I don't think you need to have the abstract check in WebObjectActivator - the inbuilt code doesn't do the check when WOA is not set - it would just throw. I would also special case public object? GetService(Type serviceType)
{
if (serviceType == typeof(IServiceProvider))
{
return (object?)serviceProvider;
}
if (serviceType == typeof(IKeyedServiceProvider))
{
return serviceProvider as IKeyedServiceProvider ?? serviceProvider.GetService(serviceType);
}
return serviceProvider.GetService(serviceType) ??
// The implementation of dependency injection in System.Web expects to be able to create instances
// of non-public and unregistered types.
Activator.CreateInstance(
serviceType,
BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.CreateInstance,
null,
null,
null);
} public static partial class HttpApplicationDependencyInjectionExtensions
{
#if NET48_OR_GREATER
public readonly static string ServiceProviderKey = typeof(IServiceProvider).FullName!;
private static IServiceProvider? GetDefaultServiceProvider() =>
HttpRuntime.WebObjectActivator?.GetService<IServiceProvider>();
public static IServiceProvider? GetServiceProvider(this HttpApplication application) =>
application.Application.GetServiceProvider() ?? GetDefaultServiceProvider();
private static IServiceProvider? GetServiceProvider(this HttpApplicationState state) =>
state[ServiceProviderKey] as IServiceProvider ?? GetDefaultServiceProvider();
public static void SetServiceProvider(this HttpApplication application, IServiceProvider serviceProvider) =>
application.Application.SetServiceProvider(serviceProvider);
public static void SetServiceProvider(this HttpApplicationState state, IServiceProvider serviceProvider)
{
if (serviceProvider is null)
{
throw new ArgumentNullException(nameof(serviceProvider));
}
state[ServiceProviderKey] = serviceProvider;
HttpRuntime.WebObjectActivator = new WebObjectActivator(serviceProvider);
}
public static void ClearServiceProvider(this HttpApplicationState state)
{
state.Remove(ServiceProviderKey);
HttpRuntime.WebObjectActivator = null;
}
#endif
#if NET8_0_OR_GREATER
public static IServiceProvider? GetServiceProvider(this HttpApplication application) =>
HttpRuntime.WebObjectActivator;
#endif
} |
src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/SystemWebAdapterExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/SystemWebAdapterExtensions.cs
Outdated
Show resolved
Hide resolved
In your using Microsoft.Extensions.Configuration;
using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Configuration;
using System.Linq;
namespace C3D.Extensions.Configuration;
public static class ConfigurationBuilderNameValueExtensions
{
public static IConfigurationBuilder AddNameValueCollection(this IConfigurationBuilder builder,
NameValueCollection collection) =>
builder.AddInMemoryCollection(
collection.AllKeys.Select(key => new KeyValuePair<string, string?>(key, collection[key])));
public static IConfigurationBuilder AddAppSettings(this IConfigurationBuilder builder) =>
builder
.AddNameValueCollection(System.Configuration.ConfigurationManager.AppSettings);
public static IConfigurationBuilder AddConnectionStrings(this IConfigurationBuilder builder,
string? providerFilter = null,
Func<ConnectionStringSettings, string>? extractConnectionString = null)
{
var connections = System.Configuration.ConfigurationManager.ConnectionStrings;
extractConnectionString ??= static cnn => cnn.ConnectionString;
return builder.AddInMemoryCollection(
connections
.OfType<ConnectionStringSettings>()
.Where(cnn => providerFilter is null || cnn.ProviderName == providerFilter)
.Select(cnn => new KeyValuePair<string, string?>("ConnectionStrings:" + cnn.Name, extractConnectionString(cnn)))
);
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AddVirtualizedContentDirectories
should probably become a <TBuilder>(this TBuilder builder) where TBuilder : IHostApplicationBuilder
style extension method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had it start off in the samples to see how useful it is
src/Microsoft.AspNetCore.SystemWebAdapters/Hosting/HostingEnvironment.cs
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,53 @@ | |||
// Licensed to the .NET Foundation under one or more agreements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would actually be really nice to have this as a separate package, perhaps with some of the DI stuff, and maybe even the ContentFile sample extensions in it, that could be used in an ASP.Net 4.x project without SWA at all.
I guess there isn't a huge overhead of pulling in the whole Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices
package, but if you simply wanted to use the aspire integration part it might be nice to have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. For now, I'm going to merge this in to be able to test it out, but agree that a separate package may make sense.
I agree the C# 14 extensions would be really nice that :( I'm not sure what you mean about "adding to the application state" - I was following the pattern of a lot of other registering patterns like bundling/routes/etc by just using a static property. Is that the same? I would imagine something like this would help surface the services/scope: using Microsoft.AspNetCore.SystemWebAdapters.Hosting;
using Microsoft.Extensions.DependencyInjection;
namespace System.Web;
public static class HttpApplicationServiceExtensions
{
private static readonly object _key = new();
public static IServiceProvider GetApplicationServices(this HttpContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
return HttpApplicationHost.Current.Services;
}
public static IServiceProvider GetRequestServices(this HttpContext context)
{
if (context is null)
{
throw new ArgumentNullException(nameof(context));
}
if (context.Items[_key] is IServiceScope existing)
{
return existing.ServiceProvider;
}
var scope = HttpApplicationHost.Current.Services.GetRequiredService<IServiceScopeFactory>().CreateScope();
context.Items[_key] = scope;
context.AddOnRequestCompleted(context =>
{
if (context.Items[_key] is IServiceScope scope)
{
scope.Dispose();
}
});
return scope.ServiceProvider;
}
} |
@@ -2,6 +2,7 @@ | |||
<Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)../'))" /> | |||
|
|||
<PropertyGroup> | |||
<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I'm following why this is now needed.
@@ -4,7 +4,7 @@ | |||
|
|||
<PropertyGroup> | |||
<OutputType>Exe</OutputType> | |||
<TargetFramework>net9.0-windows</TargetFramework> | |||
<TargetFramework>net10.0-windows</TargetFramework> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there are currently some issues people have reported around using Aspire with .NET 10. Typically most of these are during startup so if it's working well for you and things are booting up then it is likely you are not impacted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I was actually having issues in the repo with targeting net9.0-windows and net10.0-windows seems to work.
.AddAuthenticationServer(); | ||
HttpApplicationHost.RegisterHost(builder => | ||
{ | ||
builder.AddServiceDefaults(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other typical thing that is done when you add aspire to an app is to call abb.MapDefaultEndpoints() which would get things like healthchecks working. That would obviously not go here but just mentioning it in case you want to add that as well.
@@ -21,15 +27,19 @@ public static TBuilder AddServiceDefaults<TBuilder>(this TBuilder builder) where | |||
|
|||
builder.AddDefaultHealthChecks(); | |||
|
|||
#if NET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As FYI, we have an issue tracking making service discovery work against .nET Standard which would fix this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me, thanks Taylor!
This change adds a new way to configure the framework application for use with incremental migration that unlocks a lot of goodness from Aspire.
Here's an example of traces showing the incremental migration steps:
Here's a screen shot of the metrics gathered on the framework application:
This is enabled by the following:
Once these are available (and hooked up), then a lot of the APIs for the Aspire-related ServiceDefaults becomes available (at least for the stuff that supports .NET Standard/.NET Framework). To see how the samples make use of this, see:
systemweb-adapters/samples/ServiceDefaults/Extensions.cs
Lines 22 to 141 in 480f199