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

Allow IHostBuilder to have the convenient UseStartup<> pattern that IWebHostBuilder implements #42258

Closed
sonicmouse opened this issue Sep 1, 2020 · 9 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Hosting
Milestone

Comments

@sonicmouse
Copy link

Preamble

I would like to see a way to use the convenient Startup.cs pattern that IWebHostBuilder has implemented, but at the IHostBuilder level.

I don't like working in the Program.cs file... That is typically for boiler-plate code to start a host. Now I find myself in there constantly making changes just to work on my DI pipeline, for example:

Program.cs:

// ...
public static IHostBuilder CreateHostBuilder(string[] args) =>
	Host.CreateDefaultBuilder(args)
	.ConfigureServices(services =>
	{
		// add services here
	});
// ...

I'd like to see...

It would be really convenient to segregate the boiler-plate code from the DI injection routines. So instead of the code above, do this:

// ...
public static IHostBuilder CreateHostBuilder(string[] args) =>
	Host.CreateDefaultBuilder(args)
	.UseStartup<Startup>(); // new method
// ...

Additional context

I put together an extension that makes this possible, but I am not 100% sure this is proper. Maybe there is a better way you folks can come up with that would accomplish this in a more organic manner:

/// <summary>
/// Extensions to emulate a typical "Startup.cs" pattern for <see cref="IHostBuilder"/>
/// </summary>
public static class HostBuilderExtensions
{
	private const string ConfigureServicesMethodName = "ConfigureServices";

	/// <summary>
	/// Specify the startup type to be used by the host.
	/// </summary>
	/// <typeparam name="TStartup">The type containing an optional constructor with
	/// an <see cref="IConfiguration"/> parameter. The implementation should contain a public
	/// method named ConfigureServices with <see cref="IServiceCollection"/> parameter.</typeparam>
	/// <param name="hostBuilder">The <see cref="IHostBuilder"/> to initialize with TStartup.</param>
	/// <returns>The same instance of the <see cref="IHostBuilder"/> for chaining.</returns>
	public static IHostBuilder UseStartup<TStartup>(
		this IHostBuilder hostBuilder) where TStartup : class
	{
		// Invoke the ConfigureServices method on IHostBuilder...
		hostBuilder.ConfigureServices((ctx, serviceCollection) =>
		{
			// Find a method that has this signature: ConfigureServices(IServiceCollection)
			var cfgServicesMethod = typeof(TStartup).GetMethod(ConfigureServicesMethodName,
				new Type[] { typeof(IServiceCollection) });

			// Check if TStartup has a ctor that takes a IConfiguration parameter
			var hasConfigCtor = typeof(TStartup).GetConstructor(
				new Type[] { typeof(IConfiguration) }) != null;

			// create a TStartup instance based on ctor
			var startUpObj = hasConfigCtor ?
				(TStartup)Activator.CreateInstance(typeof(TStartup), ctx.Configuration) :
				(TStartup)Activator.CreateInstance(typeof(TStartup), null);

			// finally, call the ConfigureServices implemented by the TStartup object
			cfgServicesMethod?.Invoke(startUpObj, new object[] { serviceCollection });
		});

		// chain the response
		return hostBuilder;
	}
}

Thank you for taking the time to look at this request.

@KyleMit
Copy link

KyleMit commented Sep 2, 2020

+1

See also this thread on Startup.cs in a self-hosted .NET Core Console Application for a large number of use cases that would get value out of using the DI framework with the separation of concerns that comes from the UseStartup<Startup> pattern made popular in asp.net core

@BrennanConroy
Copy link
Member

UseStartup is a web app specific concern and we don't want to confuse it with the generic host.

@KyleMit
Copy link

KyleMit commented Sep 14, 2020

@BrennanConroy , not sure I understand why UseStartup is functionally scoped to just web app. The whole DI pipeline started with asp.net core and has been abstracted out for general functionality. UseStartup is just semantic sugar to initialize the host, but the methods in it don't seem specific to web apps.

@EccentricVamp
Copy link

I have to agree with @KyleMit here. This pattern has numerous use cases beyond web apps, and expanding it to other types of Core applications could help simplify and standardize service/environment configuration. The only part of it that seems "web app specific" to me would be configuration of the HTTP request pipeline, but even that has uses outside of web apps. @BrennanConroy, please open this back up. Web Apps don't have a monopoly on starting up.

@Tratcher
Copy link
Member

Tratcher commented Sep 15, 2020

I'd mainly be concerned about confusing it with existing web app Startup patterns. Let me start with some background.

Startup originated back with Microsoft.Owin and only had a Configure method for setting up an http request pipeline. AspNetCore expanded on that to add ConfigureServices for setting up DI, but the http pipeline was still the primary focus.

With IHostBuilder, ConfigureServices has moved directly to to the host builder and Configure moved to a web app specific area. The web app specific area maintained the Startup pattern for backwards compatibility, but now it's only a bridged over these two new locations. Note when we moved from WebHostBuilder to HostBuilder we lost the ability to DI inject services into the Startup class, except for a few core services provided by the HostBuilder directly. The signatures allowed for ConfigureServices also became more constrained.

The ask here is to replicate the Startup pattern for IHostBuilder, but with only a ConfigureServices method. The relevant questions are why and how?

Why?

To separate app bootstraping logic from the services logic. I can appreciate the appeal of Startup as an organizational pattern.

To inject services into Startup. This one doesn't work anymore, the only things you can inject are items from the HostBuilderContext, and you already have access to those from IHostBuilder.ConfigureServices.

How?

Adding IHostBuilder.UseStartup<T>() and similar. This one concerns me because we'd end up with two UseStartup APIs that worked very differently. If you called this new one from a web app then it wouldn't call Configure to build the request pipeline and your app would 404 (or fail to start?).

Writing code like this: hostBuilder.ConfigureServices((context, services) => new Startup(context).ConfigureServices(services));. It provides the same level of functionality but does lack some elegance.

Adding IHostBuilder.ConfigureServices<T>() to type activate the class T with the allowed host services and then call its ConfigureServices method. This would avoid confusing it with the old UseStartup pattern. Would that work for folks?

@Tratcher Tratcher reopened this Sep 15, 2020
@Tratcher Tratcher transferred this issue from dotnet/extensions Sep 15, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Extensions-Hosting untriaged New issue has not been triaged by the area owner labels Sep 15, 2020
@ghost
Copy link

ghost commented Sep 15, 2020

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

@eerhardt eerhardt added api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Sep 15, 2020
@eerhardt eerhardt added this to the Future milestone Sep 15, 2020
@eerhardt
Copy link
Member

Since this is a proposal for adding API, is someone willing make the proposal in the format of the template linked in step 1 of https://github.com/dotnet/runtime/blob/master/docs/project/api-review-process.md ?

@EccentricVamp
Copy link

Thanks @Tratcher for the thoughtful response and proposal. This solution seems adequate to me

@eerhardt I gave the template a shot #42364
I feel it is necessary to point out that I'm not the one who originally opened this request, and was merely looking for an elegant/standard solution for configuring services. So, I mostly just copied suggestions from this thread. Let me know if I messed anything up, or missed something

@eerhardt
Copy link
Member

Closing this issue in favor of #42364.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-Hosting
Projects
None yet
Development

No branches or pull requests

7 participants