Skip to content
This repository was archived by the owner on Dec 19, 2018. It is now read-only.

WIP: Simplify hosting configuration with some extension methods #991

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
3 changes: 2 additions & 1 deletion samples/SampleStartups/StartupBlockingOnStart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public static void Main(string[] args)
using (host)
{
host.Start();
Console.ReadLine();

host.WaitForShutdown();
}
}
}
Expand Down
58 changes: 45 additions & 13 deletions samples/SampleStartups/StartupHelloWorld.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,58 @@

namespace SampleStartups
{
public class StartupHelloWorld : StartupBase
public class Program
{
// This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
public override void Configure(IApplicationBuilder app)
public static void Main()
{
app.Run(async (context) =>
{
await context.Response.WriteAsync("Hello World!");
});
var host = new WebHostBuilder()
.Run(async context =>
Copy link
Member

@Tratcher Tratcher Mar 27, 2017

Choose a reason for hiding this comment

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

BuildAndStart?

Copy link
Member Author

@davidfowl davidfowl Mar 27, 2017

Choose a reason for hiding this comment

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

Too ugly. It's really BuildRunStart so no...

{
await context.Response.WriteAsync("Hello World");
});

host.WaitForShutdown();
}

public static void MainWithPort()
{
var host = new WebHostBuilder()
.Run(8080, async context =>
Copy link
Member

Choose a reason for hiding this comment

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

We've avoided specifying just a port because of the ambiguity over what the IP/Host is. Localhost? */Any? Loopack? IPv6Loopback? There's never been agreement for what the default host/ip should be.

Copy link
Member

Choose a reason for hiding this comment

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

I also don't see this extension being used outside of demoware, no real app is this simple. I'd rather have the RunApplication variant as Run.

Copy link
Member Author

Choose a reason for hiding this comment

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

We've avoided specifying just a port because of the ambiguity over what the IP/Host is. Localhost? */Any? Loopack? IPv6Loopback? There's never been agreement for what the default host/ip should be.

I know, I was there, I'm forcing the discussion by picking Any.

I also don't see this extension being used outside of demoware, no real app is this simple. I'd rather have the RunApplication variant as Run.

Agreed. Or very simple applications.

Copy link
Member

Choose a reason for hiding this comment

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

Yea this is dangerous. to me it reads like "bind to any nic, on any IP I have". That's going to be a nasty surprise for someone. Be specific here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a required hostname.

Copy link
Member

Choose a reason for hiding this comment

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

url instead of port is probably ok. UseUrls is a little bit more but having Run(...) or some similar method just take the handler instead of calling Configure is a great simplification.

{
await context.Response.WriteAsync("Hello World");
});

host.WaitForShutdown();
}

public static void MainWithMiddlewareWithPort()
{
var host = new WebHostBuilder()
.RunApplication(8080, app =>
{
// You can add middleware here
app.Run(async context =>
{
await context.Response.WriteAsync("Hello World");
});
});

host.WaitForShutdown();
}

// Entry point for the application.
public static void Main(string[] args)
public static void MainWithMiddleware()
{
var host = new WebHostBuilder()
//.UseKestrel()
.UseStartup<StartupHelloWorld>()
.Build();
.RunApplication(app =>
{
// You can add middleware here
app.Run(async context =>
{
await context.Response.WriteAsync("Hello World");
});
});

host.Run();
host.WaitForShutdown();
}
}
}
55 changes: 55 additions & 0 deletions src/Microsoft.AspNetCore.Hosting/WebHostBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Reflection;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting.Internal;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;

Expand Down Expand Up @@ -92,5 +93,59 @@ public static IWebHostBuilder UseDefaultServiceProvider(this IWebHostBuilder hos
services.Replace(ServiceDescriptor.Singleton<IServiceProviderFactory<IServiceCollection>>(new DefaultServiceProviderFactory(options)));
});
}

/// <summary>
/// Runs a web application with the specified handler
/// </summary>
/// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
/// <param name="handler">A delegate that handles the request.</param>
public static IWebHost Run(this IWebHostBuilder hostBuilder, RequestDelegate handler)
Copy link
Member

Choose a reason for hiding this comment

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

Why aren't these extensions named Start? the IWebHost.Run extensions all block but these call Start and return.

{
var host = hostBuilder.Configure(app => app.Run(handler)).Build();
host.Start();
return host;
}

/// <summary>
/// Runs a web application with the specified handler
/// </summary>
/// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
/// <param name="port">The port to bind to.</param>
/// <param name="handler">A delegate that handles the request.</param>
public static IWebHost Run(this IWebHostBuilder hostBuilder, int port, RequestDelegate handler)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't there some FxCop rule that says keep adding new parameters at the end of the method if name stays the same (agree having the port first makes more sense otherwise).

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't use FXCop 😄

{
var host = hostBuilder.UseUrls($"http://*:{port}/")
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, @blowdart has killed this before. Too many localhost test servers being publicly exposed.

Copy link
Member

Choose a reason for hiding this comment

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

No no no no no. Don't make me cry.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it has to be this way for compat with other frameworks.

golang:

For TCP and UDP, the syntax of laddr is "host:port", like "127.0.0.1:8080". If host is omitted, as in ":8080", Listen listens on all available interfaces instead of just the interface with the given host address.

nodejs

Begin accepting connections on the specified port and hostname. If the hostname is omitted, the server will accept connections directed to any IPv4 address (INADDR_ANY).

As a compromise, we could require both host and port.

Copy link
Member

Choose a reason for hiding this comment

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

We already require host and port (and scheme). Unless you're going to accept them a separate parameters I don't see this extension adding much value over UseUrls.

Copy link
Member Author

Choose a reason for hiding this comment

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

You don't but others do. Everything here is doable already this isn't about functionality. If it was we wouldn't make new APIs. It's about ease of use, API design, the right nomenclature and common parameters used to Start, Listen, Run a simple server.

Copy link
Member

Choose a reason for hiding this comment

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

Tend to agree. Taking url instead of just the port makes sense given it's consistent overall.

.Configure(app => app.Run(handler))
.Build();
host.Start();
return host;
}

/// <summary>
/// Runs a web application with the specified handler
/// </summary>
/// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
/// <param name="configure">The delegate that configures the <see cref="IApplicationBuilder"/>.</param>
public static IWebHost RunApplication(this IWebHostBuilder hostBuilder, Action<IApplicationBuilder> configure)
{
var host = hostBuilder.Configure(configure).Build();
host.Start();
return host;
}

/// <summary>
/// Runs a web application with the specified handler
/// </summary>
/// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
/// <param name="port">The port to bind to.</param>
/// <param name="configure">The delegate that configures the <see cref="IApplicationBuilder"/>.</param>
public static IWebHost RunApplication(this IWebHostBuilder hostBuilder, int port, Action<IApplicationBuilder> configure)
{
var host = hostBuilder.UseUrls($"http://*:{port}/")
.Configure(configure)
.Build();
host.Start();
return host;
}
}
}
123 changes: 76 additions & 47 deletions src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,86 @@ namespace Microsoft.AspNetCore.Hosting
{
public static class WebHostExtensions
{
/// <summary>
/// Blocks the calling thread until the web application shuts down.
/// </summary>
/// <param name="host">The <see cref="IWebHost"/> to wait for shutdown</param>
public static void WaitForShutdown(this IWebHost host)
{
WaitForSystemShutdown(host, (token, message) => host.WaitForShutdown(token, message));
}

/// <summary>
/// Blocks the calling thread until the web application shuts down.
/// </summary>
/// <param name="host">The <see cref="IWebHost"/> to wait for shutdown</param>
/// <param name="token">The token to trigger shutdown.</param>
public static void WaitForShutdown(this IWebHost host, CancellationToken token)
{
host.WaitForShutdown(token, shutdownMessage: null);
Copy link
Member Author

Choose a reason for hiding this comment

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

Should this dispose the host? Part of the API is about simplicity but it's also generally useful outside of that use case.

}

/// <summary>
/// Runs a web application and block the calling thread until host shutdown.
/// </summary>
/// <param name="host">The <see cref="IWebHost"/> to run.</param>
public static void Run(this IWebHost host)
{
WaitForSystemShutdown(host, (token, message) => host.Run(token, message));
}

/// <summary>
/// Runs a web application and block the calling thread until token is triggered or shutdown is triggered.
/// </summary>
/// <param name="host">The <see cref="IWebHost"/> to run.</param>
/// <param name="token">The token to trigger shutdown.</param>
public static void Run(this IWebHost host, CancellationToken token)
{
host.Run(token, shutdownMessage: null);
Copy link
Member

Choose a reason for hiding this comment

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

No default shutdown message?

Copy link
Member Author

Choose a reason for hiding this comment

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

This hasn't changed.

}

private static void Run(this IWebHost host, CancellationToken token, string shutdownMessage)
{
using (host)
{
host.Start();

host.WaitForShutdown(token, shutdownMessage);
}
}

private static void WaitForShutdown(this IWebHost host, CancellationToken token, string shutdownMessage)
{
var hostingEnvironment = host.Services.GetService<IHostingEnvironment>();
var applicationLifetime = host.Services.GetService<IApplicationLifetime>();

Console.WriteLine($"Hosting environment: {hostingEnvironment.EnvironmentName}");
Copy link
Member

@lutzroeder lutzroeder Mar 27, 2017

Choose a reason for hiding this comment

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

Can this be implemented without always writing to Console?

Copy link
Member Author

@davidfowl davidfowl Mar 27, 2017

Choose a reason for hiding this comment

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

There's a whole discussion on this too. I think the default should use the console. Having an options to turn it off would be fine though. See #979

Console.WriteLine($"Content root path: {hostingEnvironment.ContentRootPath}");

var serverAddresses = host.ServerFeatures.Get<IServerAddressesFeature>()?.Addresses;
if (serverAddresses != null)
{
foreach (var address in serverAddresses)
{
Console.WriteLine($"Now listening on: {address}");
}
}

if (!string.IsNullOrEmpty(shutdownMessage))
{
Console.WriteLine(shutdownMessage);
}

token.Register(state =>
{
((IApplicationLifetime)state).StopApplication();
},
applicationLifetime);

applicationLifetime.ApplicationStopping.WaitHandle.WaitOne();
}

private static void WaitForSystemShutdown(this IWebHost host, Action<CancellationToken, string> execute)
{
var done = new ManualResetEventSlim(false);
using (var cts = new CancellationTokenSource())
Expand Down Expand Up @@ -48,55 +123,9 @@ public static void Run(this IWebHost host)
eventArgs.Cancel = true;
};

host.Run(cts.Token, "Application started. Press Ctrl+C to shut down.");
execute(cts.Token, "Application started. Press Ctrl+C to shut down.");
done.Set();
}
}

/// <summary>
/// Runs a web application and block the calling thread until token is triggered or shutdown is triggered.
/// </summary>
/// <param name="host">The <see cref="IWebHost"/> to run.</param>
/// <param name="token">The token to trigger shutdown.</param>
public static void Run(this IWebHost host, CancellationToken token)
{
host.Run(token, shutdownMessage: null);
}

private static void Run(this IWebHost host, CancellationToken token, string shutdownMessage)
{
using (host)
{
host.Start();

var hostingEnvironment = host.Services.GetService<IHostingEnvironment>();
var applicationLifetime = host.Services.GetService<IApplicationLifetime>();

Console.WriteLine($"Hosting environment: {hostingEnvironment.EnvironmentName}");
Console.WriteLine($"Content root path: {hostingEnvironment.ContentRootPath}");

var serverAddresses = host.ServerFeatures.Get<IServerAddressesFeature>()?.Addresses;
if (serverAddresses != null)
{
foreach (var address in serverAddresses)
{
Console.WriteLine($"Now listening on: {address}");
}
}

if (!string.IsNullOrEmpty(shutdownMessage))
{
Console.WriteLine(shutdownMessage);
}

token.Register(state =>
{
((IApplicationLifetime)state).StopApplication();
},
applicationLifetime);

applicationLifetime.ApplicationStopping.WaitHandle.WaitOne();
}
}
}
}