huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Optimize MvcRouteHandler 

- Check MVC services once at startup
- Make action selector sync

We've never really had a scenario for the action selector being async, it
just ended up that way. None of our extensibility here lets you do
anything async without replacing it wholesale, which we don't
recommend.
This commit is contained in:
Ryan Nowak 2015-12-31 16:10:11 -08:00
parent c125b4e59b
commit 4141fcae69
5 changed files with 83 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs
View File

@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.AspNet.Mvc.ActionConstraints;
@ -16,12 +15,21 @@ using Microsoft.Extensions.Logging;
namespace Microsoft.AspNet.Mvc.Infrastructure
{
/// <summary>
/// A default <see cref="IActionSelector"/> implementation.
/// </summary>
public class DefaultActionSelector : IActionSelector
{
private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider;
private readonly IActionConstraintProvider[] _actionConstraintProviders;
private ILogger _logger;
/// <summary>
/// Creates a new <see cref="DefaultActionSelector"/>.
/// </summary>
/// <param name="decisionTreeProvider">The <see cref="IActionSelectorDecisionTreeProvider"/>.</param>
/// <param name="actionConstraintProviders">The set of <see cref="IActionInvokerProvider"/> instances.</param>
/// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
public DefaultActionSelector(
IActionSelectorDecisionTreeProvider decisionTreeProvider,
IEnumerable<IActionConstraintProvider> actionConstraintProviders,
@ -32,7 +40,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
_logger = loggerFactory.CreateLogger<DefaultActionSelector>();
}
public Task<ActionDescriptor> SelectAsync(RouteContext context)
/// <inheritdoc />
public ActionDescriptor Select(RouteContext context)
{
if (context == null)
{
@ -71,13 +80,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
if (finalMatches == null || finalMatches.Count == 0)
{
return Task.FromResult<ActionDescriptor>(null);
return null;
}
else if (finalMatches.Count == 1)
{
var selectedAction = finalMatches[0];
return Task.FromResult(selectedAction);
return selectedAction;
}
else
{

14
src/Microsoft.AspNet.Mvc.Core/Infrastructure/IActionSelector.cs
View File

@ -1,14 +1,24 @@
// 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.Threading.Tasks;
using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.AspNet.Routing;
namespace Microsoft.AspNet.Mvc.Infrastructure
{
/// <summary>
/// Defines an interface for selecting an MVC action to invoke for the current request.
/// </summary>
public interface IActionSelector
{
Task<ActionDescriptor> SelectAsync(RouteContext context);
/// <summary>
/// Selects an <see cref="ActionDescriptor"/> for the request associated with <paramref name="context"/>.
/// </summary>
/// <param name="context">The <see cref="RouteContext"/> for the current request.</param>
/// <returns>An <see cref="ActionDescriptor"/> or <c>null</c> if no action can be selected.</returns>
/// <exception cref="AmbiguousActionException">
/// Thrown when action selection results in an ambiguity.
/// </exception>
ActionDescriptor Select(RouteContext context);
}
}

34
src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs
View File

@ -38,25 +38,20 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
return null;
}
public async Task RouteAsync(RouteContext context)
public Task RouteAsync(RouteContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}
var services = context.HttpContext.RequestServices;
// Verify if AddMvc was done before calling UseMvc
// We use the MvcMarkerService to make sure if all the services were added.
MvcServicesHelper.ThrowIfMvcNotRegistered(services);
EnsureServices(context.HttpContext);
var actionDescriptor = await _actionSelector.SelectAsync(context);
var actionDescriptor = _actionSelector.Select(context);
if (actionDescriptor == null)
{
_logger.NoActionsMatched();
return;
return TaskCache.CompletedTask;
}
if (actionDescriptor.RouteValueDefaults != null)
@ -68,12 +63,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
context.RouteData.Values.Add(kvp.Key, kvp.Value);
}
}
// Removing RouteGroup from RouteValues to simulate the result of conventional routing
context.RouteData.Values.Remove(TreeRouter.RouteGroupKey);
}
// Removing RouteGroup from RouteValues to simulate the result of conventional routing
context.RouteData.Values.Remove(TreeRouter.RouteGroupKey);
context.Handler = (c) => InvokeActionAsync(c, actionDescriptor);
return TaskCache.CompletedTask;
}
private async Task InvokeActionAsync(HttpContext httpContext, ActionDescriptor actionDescriptor)
@ -121,15 +117,21 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
return;
}
var services = context.RequestServices;
// Verify if AddMvc was done before calling UseMvc
// We use the MvcMarkerService to make sure if all the services were added.
MvcServicesHelper.ThrowIfMvcNotRegistered(services);
// The IActionContextAccessor is optional. We want to avoid the overhead of using CallContext
// if possible.
_actionContextAccessor = context.RequestServices.GetService<IActionContextAccessor>();
_actionContextAccessor = services.GetService<IActionContextAccessor>();
_actionInvokerFactory = context.RequestServices.GetRequiredService<IActionInvokerFactory>();
_actionSelector = context.RequestServices.GetRequiredService<IActionSelector>();
_diagnosticSource = context.RequestServices.GetRequiredService<DiagnosticSource>();
_actionInvokerFactory = services.GetRequiredService<IActionInvokerFactory>();
_actionSelector = services.GetRequiredService<IActionSelector>();
_diagnosticSource = services.GetRequiredService<DiagnosticSource>();
var factory = context.RequestServices.GetRequiredService<ILoggerFactory>();
var factory = services.GetRequiredService<ILoggerFactory>();
_logger = factory.CreateLogger<MvcRouteHandler>();
_servicesRetrieved = true;

74
test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs
View File

@ -5,7 +5,6 @@ using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNet.Http;
using Microsoft.AspNet.Http.Internal;
using Microsoft.AspNet.Mvc.Abstractions;
@ -25,7 +24,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
public class DefaultActionSelectorTests
{
[Fact]
public async void SelectAsync_AmbiguousActions_LogIsCorrect()
public void Select_AmbiguousActions_LogIsCorrect()
{
// Arrange
var sink = new TestSink();
@ -44,10 +43,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
$"ambiguity. Matching actions: {actionNames}";
// Act
await Assert.ThrowsAsync<AmbiguousActionException>(async () =>
{
await selector.SelectAsync(routeContext);
});
Assert.Throws<AmbiguousActionException>(() => { selector.Select(routeContext); });
// Assert
Assert.Empty(sink.Scopes);
@ -56,7 +52,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
}
[Fact]
public async Task SelectAsync_PrefersActionWithConstraints()
public void Select_PrefersActionWithConstraints()
{
// Arrange
var actionWithConstraints = new ActionDescriptor()
@ -79,14 +75,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, actionWithConstraints);
}
[Fact]
public async Task SelectAsync_ConstraintsRejectAll()
public void Select_ConstraintsRejectAll()
{
// Arrange
var action1 = new ActionDescriptor()
@ -111,14 +107,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Null(action);
}
[Fact]
public async Task SelectAsync_ConstraintsRejectAll_DifferentStages()
public void Select_ConstraintsRejectAll_DifferentStages()
{
// Arrange
var action1 = new ActionDescriptor()
@ -145,14 +141,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Null(action);
}
[Fact]
public async Task SelectAsync_ActionConstraintFactory()
public void Select_ActionConstraintFactory()
{
// Arrange
var actionWithConstraints = new ActionDescriptor()
@ -177,14 +173,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, actionWithConstraints);
}
[Fact]
public async Task SelectAsync_ActionConstraintFactory_ReturnsNull()
public void Select_ActionConstraintFactory_ReturnsNull()
{
// Arrange
var nullConstraint = new ActionDescriptor()
@ -203,7 +199,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, nullConstraint);
@ -211,7 +207,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
// There's a custom constraint provider registered that only understands BooleanConstraintMarker
[Fact]
public async Task SelectAsync_CustomProvider()
public void Select_CustomProvider()
{
// Arrange
var actionWithConstraints = new ActionDescriptor()
@ -233,7 +229,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, actionWithConstraints);
@ -241,7 +237,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
// Due to ordering of stages, the first action will be better.
[Fact]
public async Task SelectAsync_ConstraintsInOrder()
public void Select_ConstraintsInOrder()
{
// Arrange
var best = new ActionDescriptor()
@ -266,7 +262,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, best);
@ -274,7 +270,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
// Due to ordering of stages, the first action will be better.
[Fact]
public async Task SelectAsync_ConstraintsInOrder_MultipleStages()
public void Select_ConstraintsInOrder_MultipleStages()
{
// Arrange
var best = new ActionDescriptor()
@ -303,14 +299,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, best);
}
[Fact]
public async Task SelectAsync_Fallback_ToActionWithoutConstraints()
public void Select_Fallback_ToActionWithoutConstraints()
{
// Arrange
var nomatch1 = new ActionDescriptor()
@ -341,14 +337,14 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var context = CreateRouteContext("POST");
// Act
var action = await selector.SelectAsync(context);
var action = selector.Select(context);
// Assert
Assert.Same(action, best);
}
[Fact]
public async Task SelectAsync_Ambiguous()
public void Select_Ambiguous()
{
// Arrange
var expectedMessage =
@ -375,9 +371,9 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
context.RouteData.Values.Add("action", "Buy");
// Act
var ex = await Assert.ThrowsAsync<AmbiguousActionException>(async () =>
var ex = Assert.Throws<AmbiguousActionException>(() =>
{
await selector.SelectAsync(context);
selector.Select(context);
});
// Assert
@ -390,7 +386,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
[InlineData("POST")]
[InlineData("DELETE")]
[InlineData("PATCH")]
public async Task HttpMethodAttribute_ActionWithMultipleHttpMethodAttributeViaAcceptVerbs_ORsMultipleHttpMethods(string verb)
public void HttpMethodAttribute_ActionWithMultipleHttpMethodAttributeViaAcceptVerbs_ORsMultipleHttpMethods(string verb)
{
// Arrange
var routeContext = new RouteContext(GetHttpContext(verb));
@ -398,7 +394,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
routeContext.RouteData.Values.Add("action", "Patch");
// Act
var result = await InvokeActionSelector(routeContext);
var result = InvokeActionSelector(routeContext);
// Assert
Assert.Equal("Patch", result.Name);
@ -411,7 +407,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
[InlineData("DELETE")]
[InlineData("PATCH")]
[InlineData("HEAD")]
public async Task HttpMethodAttribute_ActionWithMultipleHttpMethodAttributes_ORsMultipleHttpMethods(string verb)
public void HttpMethodAttribute_ActionWithMultipleHttpMethodAttributes_ORsMultipleHttpMethods(string verb)
{
// Arrange
var routeContext = new RouteContext(GetHttpContext(verb));
@ -419,7 +415,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
routeContext.RouteData.Values.Add("action", "Put");
// Act
var result = await InvokeActionSelector(routeContext);
var result = InvokeActionSelector(routeContext);
// Assert
Assert.Equal("Put", result.Name);
@ -428,7 +424,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
[Theory]
[InlineData("GET")]
[InlineData("PUT")]
public async Task HttpMethodAttribute_ActionDecoratedWithHttpMethodAttribute_OverridesConvention(string verb)
public void HttpMethodAttribute_ActionDecoratedWithHttpMethodAttribute_OverridesConvention(string verb)
{
// Arrange
// Note no action name is passed, hence should return a null action descriptor.
@ -436,7 +432,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
routeContext.RouteData.Values.Add("controller", "HttpMethodAttributeTests_RestOnly");
// Act
var result = await InvokeActionSelector(routeContext);
var result = InvokeActionSelector(routeContext);
// Assert
Assert.Null(result);
@ -466,7 +462,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
[InlineData("POST")]
[InlineData("DELETE")]
[InlineData("PATCH")]
public async Task ActionNameAttribute_ActionGetsExposedViaActionName_UnreachableByConvention(string verb)
public void ActionNameAttribute_ActionGetsExposedViaActionName_UnreachableByConvention(string verb)
{
// Arrange
var routeContext = new RouteContext(GetHttpContext(verb));
@ -474,7 +470,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
routeContext.RouteData.Values.Add("action", "RPCMethodWithHttpGet");
// Act
var result = await InvokeActionSelector(routeContext);
var result = InvokeActionSelector(routeContext);
// Assert
Assert.Null(result);
@ -496,7 +492,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
[InlineData("POST", "CustomActionName_RpcMethod")]
[InlineData("DELETE", "CustomActionName_RpcMethod")]
[InlineData("PATCH", "CustomActionName_RpcMethod")]
public async Task ActionNameAttribute_DifferentActionName_UsesActionNameFromActionNameAttribute(string verb, string actionName)
public void ActionNameAttribute_DifferentActionName_UsesActionNameFromActionNameAttribute(string verb, string actionName)
{
// Arrange
var routeContext = new RouteContext(GetHttpContext(verb));
@ -504,13 +500,13 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
routeContext.RouteData.Values.Add("action", actionName);
// Act
var result = await InvokeActionSelector(routeContext);
var result = InvokeActionSelector(routeContext);
// Assert
Assert.Equal(actionName, result.Name);
}
private async Task<ActionDescriptor> InvokeActionSelector(RouteContext context)
private ActionDescriptor InvokeActionSelector(RouteContext context)
{
var actionDescriptorProvider = GetActionDescriptorProvider();
@ -536,7 +532,7 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
actionConstraintProviders,
NullLoggerFactory.Instance);
return await defaultActionSelector.SelectAsync(context);
return defaultActionSelector.Select(context);
}
private ControllerActionDescriptorProvider GetActionDescriptorProvider()

9
test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs
View File

@ -58,8 +58,9 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
var loggerFactory = new TestLoggerFactory(sink, enabled: true);
var mockActionSelector = new Mock<IActionSelector>();
mockActionSelector.Setup(a => a.SelectAsync(It.IsAny<RouteContext>()))
.Returns(Task.FromResult<ActionDescriptor>(null));
mockActionSelector
.Setup(a => a.Select(It.IsAny<RouteContext>()))
.Returns<ActionDescriptor>(null);
var context = CreateRouteContext(
actionSelector: mockActionSelector.Object,
@ -172,8 +173,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
if (actionSelector == null)
{
var mockActionSelector = new Mock<IActionSelector>();
mockActionSelector.Setup(a => a.SelectAsync(It.IsAny<RouteContext>()))
.Returns(Task.FromResult(actionDescriptor));
mockActionSelector.Setup(a => a.Select(It.IsAny<RouteContext>()))
.Returns(actionDescriptor);
actionSelector = mockActionSelector.Object;
}