Addressed code review comments.

This commit is contained in:
N. Taylor Mullen 2019-07-19 10:29:03 -07:00
parent 9a6881b0e8
commit 479d5ed40a
2 changed files with 44 additions and 4 deletions

View File

@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.Routing
{
Log.MatchSkipped(_logger, endpoint);
// Someone else set the endpoint, we'll let them handle the unsetting.
// Someone else set the endpoint, we'll let them handle the clearing of the endpoint.
return _next(httpContext);
}
@ -88,7 +88,6 @@ namespace Microsoft.AspNetCore.Routing
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private async Task SetRoutingAndContinue(HttpContext httpContext)
{
// If there was no mutation of the endpoint then log failure
@ -115,8 +114,7 @@ namespace Microsoft.AspNetCore.Routing
}
finally
{
// We unset the endpoint after calling through to the next middleware. This enables any future calls into
// endpoint routing don't no-op from there already being an endpoint set.
// This allows a second call in a single request (such as from the ErrorHandlerMiddleware) to perform routing again.
httpContext.SetEndpoint(endpoint: null);
}
}

View File

@ -20,6 +20,48 @@ namespace Microsoft.AspNetCore.Routing
{
public class EndpointRoutingMiddlewareTest
{
[Fact]
public async Task Invoke_ChangedPath_ResultsInDifferentResult()
{
// Arrange
var httpContext = CreateHttpContext();
var matcher = new Mock<Matcher>();
var pathToEndpoints = new Dictionary<string, Endpoint>()
{
["/initial"] = new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(), "initialEndpoint"),
["/changed"] = new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(), "changedEndpoint")
};
matcher.Setup(m => m.MatchAsync(httpContext))
.Callback<HttpContext>(context =>
{
var endpointToSet = pathToEndpoints[context.Request.Path];
context.SetEndpoint(endpointToSet);
})
.Returns(Task.CompletedTask)
.Verifiable();
var matcherFactory = Mock.Of<MatcherFactory>(factory => factory.CreateMatcher(It.IsAny<EndpointDataSource>()) == matcher.Object);
var middleware = CreateMiddleware(
matcherFactory: matcherFactory,
next: context =>
{
Assert.True(pathToEndpoints.TryGetValue(context.Request.Path, out var expectedEndpoint));
var currentEndpoint = context.GetEndpoint();
Assert.Equal(expectedEndpoint, currentEndpoint);
return Task.CompletedTask;
});
// Act
httpContext.Request.Path = "/initial";
await middleware.Invoke(httpContext);
httpContext.Request.Path = "/changed";
await middleware.Invoke(httpContext);
// Assert
matcher.Verify();
}
[Fact]
public async Task Invoke_OnException_ResetsEndpoint()
{