Skip to content

Remove async state machines from middlewares when they do nothing #34581

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

Merged
merged 2 commits into from
Jul 23, 2021
Merged

Remove async state machines from middlewares when they do nothing #34581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
aef4e98
Remove async state machines from middlewares when they do nothing
campersau Jul 21, 2021
ca4bbc4
PR feedback
campersau Jul 22, 2021
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
42 changes: 21 additions & 21 deletions src/Http/Http.Abstractions/src/Extensions/MapMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public MapMiddleware(RequestDelegate next, MapOptions options)
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
/// <returns>A task that represents the execution of this middleware.</returns>
public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (context == null)
{
Expand All @@ -55,32 +55,32 @@ public async Task Invoke(HttpContext context)

if (context.Request.Path.StartsWithSegments(_options.PathMatch, out var matchedPath, out var remainingPath))
{
var path = context.Request.Path;
var pathBase = context.Request.PathBase;

if (!_options.PreserveMatchedPathSegment)
{
// Update the path
context.Request.PathBase = pathBase.Add(matchedPath);
context.Request.Path = remainingPath;
return InvokeCore(context, matchedPath, remainingPath);
}
return _options.Branch!(context);
}
return _next(context);
}

try
{
await _options.Branch!(context);
}
finally
{
if (!_options.PreserveMatchedPathSegment)
{
context.Request.PathBase = pathBase;
context.Request.Path = path;
}
}
private async Task InvokeCore(HttpContext context, string matchedPath, string remainingPath)
{
var path = context.Request.Path;
var pathBase = context.Request.PathBase;

// Update the path
context.Request.PathBase = pathBase.Add(matchedPath);
context.Request.Path = remainingPath;

try
{
await _options.Branch!(context);
}
else
finally
{
await _next(context);
context.Request.PathBase = pathBase;
context.Request.Path = path;
}
}
}
Expand Down
9 changes: 3 additions & 6 deletions src/Http/Http.Abstractions/src/Extensions/MapWhenMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public MapWhenMiddleware(RequestDelegate next, MapWhenOptions options)
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
/// <returns>A task that represents the execution of this middleware.</returns>
public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (context == null)
{
Expand All @@ -60,12 +60,9 @@ public async Task Invoke(HttpContext context)

if (_options.Predicate!(context))
{
await _options.Branch!(context);
}
else
{
await _next(context);
return _options.Branch!(context);
}
return _next(context);
}
}
}
43 changes: 21 additions & 22 deletions src/Http/Http.Abstractions/src/Extensions/UsePathBaseMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
Expand Down Expand Up @@ -41,37 +41,36 @@ public UsePathBaseMiddleware(RequestDelegate next, PathString pathBase)
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
/// <returns>A task that represents the execution of this middleware.</returns>
public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

PathString matchedPath;
PathString remainingPath;

if (context.Request.Path.StartsWithSegments(_pathBase, out matchedPath, out remainingPath))
if (context.Request.Path.StartsWithSegments(_pathBase, out var matchedPath, out var remainingPath))
{
var originalPath = context.Request.Path;
var originalPathBase = context.Request.PathBase;
context.Request.Path = remainingPath;
context.Request.PathBase = originalPathBase.Add(matchedPath);

try
{
await _next(context);
}
finally
{
context.Request.Path = originalPath;
context.Request.PathBase = originalPathBase;
}
return InvokeCore(context, matchedPath, remainingPath);
}
else
return _next(context);
}

private async Task InvokeCore(HttpContext context, string matchedPath, string remainingPath)
Copy link
Member

Choose a reason for hiding this comment

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

In Kestrel we often use the "Awaited" suffix to indicate why we're using this patter. So this would be InvokeAwaited. I'm okay with taking this as-is though.

{
var originalPath = context.Request.Path;
var originalPathBase = context.Request.PathBase;
context.Request.Path = remainingPath;
context.Request.PathBase = originalPathBase.Add(matchedPath);

try
{
await _next(context);
}
finally
{
context.Request.Path = originalPath;
context.Request.PathBase = originalPathBase;
}
}
}
}
}
50 changes: 26 additions & 24 deletions src/Middleware/Diagnostics.EntityFrameworkCore/src/MigrationsEndPointMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public MigrationsEndPointMiddleware(
/// </summary>
/// <param name="context">The context for the current request.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
public virtual async Task Invoke(HttpContext context)
public virtual Task Invoke(HttpContext context)
{
if (context == null)
{
Expand All @@ -68,39 +68,41 @@ public virtual async Task Invoke(HttpContext context)

if (context.Request.Path.Equals(_options.Path))
{
_logger.RequestPathMatched(context.Request.Path);
return InvokeCore(context);
}
return _next(context);
}

var db = await GetDbContext(context, _logger);
private async Task InvokeCore(HttpContext context)
{
_logger.RequestPathMatched(context.Request.Path);

if (db != null)
var db = await GetDbContext(context, _logger);

if (db != null)
{
var dbName = db.GetType().FullName!;
try
{
var dbName = db.GetType().FullName!;
try
{
_logger.ApplyingMigrations(dbName);
_logger.ApplyingMigrations(dbName);

await db.Database.MigrateAsync();
await db.Database.MigrateAsync();

context.Response.StatusCode = (int)HttpStatusCode.NoContent;
context.Response.Headers.Add("Pragma", new[] { "no-cache" });
context.Response.Headers.Add("Cache-Control", new[] { "no-cache,no-store" });
context.Response.StatusCode = (int)HttpStatusCode.NoContent;
context.Response.Headers.Add("Pragma", new[] { "no-cache" });
context.Response.Headers.Add("Cache-Control", new[] { "no-cache,no-store" });

_logger.MigrationsApplied(dbName);
}
catch (Exception ex)
{
var message = Strings.FormatMigrationsEndPointMiddleware_Exception(dbName) + ex;
_logger.MigrationsApplied(dbName);
}
catch (Exception ex)
{
var message = Strings.FormatMigrationsEndPointMiddleware_Exception(dbName) + ex;

_logger.MigrationsEndPointMiddlewareException(dbName, ex);
_logger.MigrationsEndPointMiddlewareException(dbName, ex);

throw new InvalidOperationException(message, ex);
}
throw new InvalidOperationException(message, ex);
}
}
else
{
await _next(context);
}
}

private static async Task<DbContext?> GetDbContext(HttpContext context, ILogger logger)
Expand Down
20 changes: 13 additions & 7 deletions src/Middleware/HttpOverrides/src/HttpMethodOverrideMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,15 @@ public HttpMethodOverrideMiddleware(RequestDelegate next, IOptions<HttpMethodOve
/// Executes the middleware.
/// </summary>
/// <param name="context">The <see cref="HttpContext"/> for the current request.</param>
public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (HttpMethods.IsPost(context.Request.Method))
{
if (_options.FormFieldName != null)
{
if (context.Request.HasFormContentType)
{
var form = await context.Request.ReadFormAsync();
var methodType = form[_options.FormFieldName];
if (!string.IsNullOrEmpty(methodType))
{
context.Request.Method = methodType;
}
return InvokeCore(context);
}
}
else
Expand All @@ -66,6 +61,17 @@ public async Task Invoke(HttpContext context)
}
}
}
return _next(context);
}

private async Task InvokeCore(HttpContext context)
{
var form = await context.Request.ReadFormAsync();
var methodType = form[_options.FormFieldName!];
if (!string.IsNullOrEmpty(methodType))
{
context.Request.Method = methodType;
}
await _next(context);
}
}
Expand Down
9 changes: 6 additions & 3 deletions src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,17 @@ public ResponseCompressionMiddleware(RequestDelegate next, IResponseCompressionP
/// </summary>
/// <param name="context">The <see cref="HttpContext"/>.</param>
/// <returns>A task that represents the execution of this middleware.</returns>
public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (!_provider.CheckRequestAcceptsCompression(context))
{
await _next(context);
return;
return _next(context);
}
return InvokeCore(context);
}

private async Task InvokeCore(HttpContext context)
{
var originalBodyFeature = context.Features.Get<IHttpResponseBodyFeature>();
var originalCompressionFeature = context.Features.Get<IHttpsCompressionFeature>();

Expand Down
34 changes: 18 additions & 16 deletions src/Middleware/Spa/SpaProxy/src/SpaProxyMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,29 +45,31 @@ public SpaProxyMiddleware(
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
}

public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (context.Request.Path.Equals(new Uri(_options.Value.ServerUrl).LocalPath))
{
context.Response.Headers[HeaderNames.CacheControl] = "no-cache, no-store, must-revalidate, max-age=0";
if (!await _spaProxyLaunchManager.IsSpaProxyRunning(context.RequestAborted))
{
_spaProxyLaunchManager.StartInBackground(_hostLifetime.ApplicationStopping);
_logger.LogInformation("SPA proxy is not ready. Returning temporary landing page.");
context.Response.ContentType = "text/html";
return InvokeCore(context);
}
return _next(context);
}

private async Task InvokeCore(HttpContext context)
{
context.Response.Headers[HeaderNames.CacheControl] = "no-cache, no-store, must-revalidate, max-age=0";
if (!await _spaProxyLaunchManager.IsSpaProxyRunning(context.RequestAborted))
{
_spaProxyLaunchManager.StartInBackground(_hostLifetime.ApplicationStopping);
_logger.LogInformation("SPA proxy is not ready. Returning temporary landing page.");
context.Response.ContentType = "text/html";

await using var writer = new StreamWriter(context.Response.Body, Encoding.UTF8);
await writer.WriteAsync(GenerateSpaLaunchPage(_options.Value));
}
else
{
_logger.LogInformation($"SPA proxy is ready. Redirecting to {_options.Value.ServerUrl}.");
context.Response.Redirect(_options.Value.ServerUrl);
}
await using var writer = new StreamWriter(context.Response.Body, Encoding.UTF8);
await writer.WriteAsync(GenerateSpaLaunchPage(_options.Value));
}
else
{
await _next(context);
_logger.LogInformation($"SPA proxy is ready. Redirecting to {_options.Value.ServerUrl}.");
context.Response.Redirect(_options.Value.ServerUrl);
}

string GenerateSpaLaunchPage(SpaDevelopmentServerOptions options)
Expand Down
20 changes: 13 additions & 7 deletions src/Middleware/Spa/SpaServices.Extensions/src/Proxying/ConditionalProxyMiddleware.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,16 +43,22 @@ public ConditionalProxyMiddleware(
_applicationStoppingToken = applicationLifetime.ApplicationStopping;
}

public async Task Invoke(HttpContext context)
public Task Invoke(HttpContext context)
{
if (context.Request.Path.StartsWithSegments(_pathPrefix) || _pathPrefixIsRoot)
{
var didProxyRequest = await SpaProxy.PerformProxyRequest(
context, _httpClient, _baseUriTask, _applicationStoppingToken, proxy404s: false);
if (didProxyRequest)
{
return;
}
return InvokeCore(context);
}
return _next.Invoke(context);
}

private async Task InvokeCore(HttpContext context)
{
var didProxyRequest = await SpaProxy.PerformProxyRequest(
context, _httpClient, _baseUriTask, _applicationStoppingToken, proxy404s: false);
if (didProxyRequest)
{
return;
}

// Not a request we can proxy
Expand Down
Loading