diff --git a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs index fb04068fb6..ae692697e0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/DefaultActionSelector.cs @@ -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 { + /// + /// A default implementation. + /// public class DefaultActionSelector : IActionSelector { private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider; private readonly IActionConstraintProvider[] _actionConstraintProviders; private ILogger _logger; + /// + /// Creates a new . + /// + /// The . + /// The set of instances. + /// The . public DefaultActionSelector( IActionSelectorDecisionTreeProvider decisionTreeProvider, IEnumerable actionConstraintProviders, @@ -32,7 +40,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure _logger = loggerFactory.CreateLogger(); } - public Task SelectAsync(RouteContext context) + /// + 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(null); + return null; } else if (finalMatches.Count == 1) { var selectedAction = finalMatches[0]; - return Task.FromResult(selectedAction); + return selectedAction; } else { diff --git a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/IActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/IActionSelector.cs index 7fd3d18f0e..80679c79be 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/IActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/IActionSelector.cs @@ -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 { + /// + /// Defines an interface for selecting an MVC action to invoke for the current request. + /// public interface IActionSelector { - Task SelectAsync(RouteContext context); + /// + /// Selects an for the request associated with . + /// + /// The for the current request. + /// An or null if no action can be selected. + /// + /// Thrown when action selection results in an ambiguity. + /// + ActionDescriptor Select(RouteContext context); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs index 0b4a44d334..da8686b3e2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Infrastructure/MvcRouteHandler.cs @@ -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(); + _actionContextAccessor = services.GetService(); - _actionInvokerFactory = context.RequestServices.GetRequiredService(); - _actionSelector = context.RequestServices.GetRequiredService(); - _diagnosticSource = context.RequestServices.GetRequiredService(); + _actionInvokerFactory = services.GetRequiredService(); + _actionSelector = services.GetRequiredService(); + _diagnosticSource = services.GetRequiredService(); - var factory = context.RequestServices.GetRequiredService(); + var factory = services.GetRequiredService(); _logger = factory.CreateLogger(); _servicesRetrieved = true; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs index a4221d16c4..0e63851ffe 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs @@ -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(async () => - { - await selector.SelectAsync(routeContext); - }); + Assert.Throws(() => { 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(async () => + var ex = Assert.Throws(() => { - 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 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() diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs index 45fd619168..ec6665eeb6 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs @@ -58,8 +58,9 @@ namespace Microsoft.AspNet.Mvc.Infrastructure var loggerFactory = new TestLoggerFactory(sink, enabled: true); var mockActionSelector = new Mock(); - mockActionSelector.Setup(a => a.SelectAsync(It.IsAny())) - .Returns(Task.FromResult(null)); + mockActionSelector + .Setup(a => a.Select(It.IsAny())) + .Returns(null); var context = CreateRouteContext( actionSelector: mockActionSelector.Object, @@ -172,8 +173,8 @@ namespace Microsoft.AspNet.Mvc.Infrastructure if (actionSelector == null) { var mockActionSelector = new Mock(); - mockActionSelector.Setup(a => a.SelectAsync(It.IsAny())) - .Returns(Task.FromResult(actionDescriptor)); + mockActionSelector.Setup(a => a.Select(It.IsAny())) + .Returns(actionDescriptor); actionSelector = mockActionSelector.Object; }