diff --git a/src/Microsoft.AspNet.Mvc.Core/AmbiguousActionException.cs b/src/Microsoft.AspNet.Mvc.Core/AmbiguousActionException.cs new file mode 100644 index 0000000000..c9e7f7f0fc --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/AmbiguousActionException.cs @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Runtime.Serialization; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// An exception which indicates multiple matches in action selection. + /// + #if ASPNET50 + [Serializable] + #endif + public class AmbiguousActionException : InvalidOperationException + { + public AmbiguousActionException(string message) + : base(message) + { + } + + #if ASPNET50 + protected AmbiguousActionException(SerializationInfo info, StreamingContext context) + : base(info, context) + { + } + #endif + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs index 0713d78cf8..eaa181046a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Net; using System.Reflection; using Microsoft.AspNet.Mvc.Routing; @@ -12,26 +11,6 @@ namespace Microsoft.AspNet.Mvc { public class DefaultActionDiscoveryConventions : IActionDiscoveryConventions { - private static readonly string[] _supportedHttpMethodsByConvention = - { - "GET", - "POST", - "PUT", - "DELETE", - "PATCH", - }; - - private static readonly string[] _supportedHttpMethodsForDefaultMethod = - { - "GET", - "POST" - }; - - public virtual string DefaultMethodName - { - get { return "Index"; } - } - public virtual bool IsController([NotNull] TypeInfo typeInfo) { if (!typeInfo.IsClass || @@ -73,17 +52,20 @@ namespace Microsoft.AspNet.Mvc } else { - actionInfos = GetActionsForMethodsWithoutCustomAttributes(methodInfo, controllerTypeInfo); + // By default the action is just matched by name. + actionInfos = new ActionInfo[] + { + new ActionInfo() + { + ActionName = methodInfo.Name, + RequireActionNameMatch = true, + } + }; } return actionInfos; } - protected virtual bool IsDefaultActionMethod([NotNull] MethodInfo methodInfo) - { - return String.Equals(methodInfo.Name, DefaultMethodName, StringComparison.OrdinalIgnoreCase); - } - /// /// Determines whether the method is a valid action. /// @@ -107,18 +89,6 @@ namespace Microsoft.AspNet.Mvc method.GetBaseDefinition().DeclaringType != typeof(object); } - public virtual IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) - { - var supportedHttpMethods = - _supportedHttpMethodsByConvention.FirstOrDefault( - httpMethod => methodInfo.Name.Equals(httpMethod, StringComparison.OrdinalIgnoreCase)); - - if (supportedHttpMethods != null) - { - yield return supportedHttpMethods; - } - } - private bool HasCustomAttributes(MethodInfo methodInfo) { var actionAttributes = GetActionCustomAttributes(methodInfo); @@ -256,70 +226,6 @@ namespace Microsoft.AspNet.Mvc return null; } - private IEnumerable GetActionsForMethodsWithoutCustomAttributes( - MethodInfo methodInfo, - TypeInfo controllerTypeInfo) - { - var actionInfos = new List(); - var httpMethods = GetSupportedHttpMethods(methodInfo); - if (httpMethods != null && httpMethods.Any()) - { - return new[] - { - new ActionInfo() - { - HttpMethods = httpMethods.ToArray(), - ActionName = methodInfo.Name, - RequireActionNameMatch = false, - } - }; - } - - // For Default Method add an action Info with GET, POST Http Method constraints. - // Only constraints (out of GET and POST) for which there are no convention based actions available are - // added. If there are existing action infos with http constraints for GET and POST, this action info is - // not added for default method. - if (IsDefaultActionMethod(methodInfo)) - { - var existingHttpMethods = new HashSet(); - foreach (var declaredMethodInfo in controllerTypeInfo.DeclaredMethods) - { - if (!IsValidActionMethod(declaredMethodInfo) || HasCustomAttributes(declaredMethodInfo)) - { - continue; - } - - httpMethods = GetSupportedHttpMethods(declaredMethodInfo); - if (httpMethods != null) - { - existingHttpMethods.UnionWith(httpMethods); - } - } - var undefinedHttpMethods = _supportedHttpMethodsForDefaultMethod.Except( - existingHttpMethods, - StringComparer.Ordinal) - .ToArray(); - if (undefinedHttpMethods.Any()) - { - actionInfos.Add(new ActionInfo() - { - HttpMethods = undefinedHttpMethods, - ActionName = methodInfo.Name, - RequireActionNameMatch = false, - }); - } - } - - actionInfos.Add( - new ActionInfo() - { - ActionName = methodInfo.Name, - RequireActionNameMatch = true, - }); - - return actionInfos; - } - private class ActionAttributes { public ActionNameAttribute ActionNameAttribute { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs index f762dcacdf..b018b24e20 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.Logging; -using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Routing; using Microsoft.Framework.Logging; @@ -18,22 +17,19 @@ namespace Microsoft.AspNet.Mvc { private readonly IActionDescriptorsCollectionProvider _actionDescriptorsCollectionProvider; private readonly IActionSelectorDecisionTreeProvider _decisionTreeProvider; - private readonly IActionBindingContextProvider _bindingProvider; private ILogger _logger; public DefaultActionSelector( [NotNull] IActionDescriptorsCollectionProvider actionDescriptorsCollectionProvider, - [NotNull] IActionSelectorDecisionTreeProvider decisionTreeProvider, - [NotNull] IActionBindingContextProvider bindingProvider, + [NotNull] IActionSelectorDecisionTreeProvider decisionTreeProvider, [NotNull] ILoggerFactory loggerFactory) { _actionDescriptorsCollectionProvider = actionDescriptorsCollectionProvider; _decisionTreeProvider = decisionTreeProvider; - _bindingProvider = bindingProvider; _logger = loggerFactory.Create(); } - public async Task SelectAsync([NotNull] RouteContext context) + public Task SelectAsync([NotNull] RouteContext context) { using (_logger.BeginScope("DefaultActionSelector.SelectAsync")) { @@ -67,7 +63,9 @@ namespace Microsoft.AspNet.Mvc matching = matchesWithConstraints; } - if (matching.Count == 0) + var finalMatches = SelectBestActions(matching); + + if (finalMatches.Count == 0) { if (_logger.IsEnabled(TraceType.Information)) { @@ -75,17 +73,17 @@ namespace Microsoft.AspNet.Mvc { ActionsMatchingRouteConstraints = matchingRouteConstraints, ActionsMatchingRouteAndMethodConstraints = matchingRouteAndMethodConstraints, - ActionsMatchingRouteAndMethodAndDynamicConstraints = + ActionsMatchingRouteAndMethodAndDynamicConstraints = matchingRouteAndMethodAndDynamicConstraints, - ActionsMatchingWithConstraints = matchesWithConstraints + FinalMatches = finalMatches, }); } - return null; + return Task.FromResult(null); } - else + else if (finalMatches.Count == 1) { - var selectedAction = await SelectBestCandidate(context, matching); + var selectedAction = finalMatches[0]; if (_logger.IsEnabled(TraceType.Information)) { @@ -95,16 +93,50 @@ namespace Microsoft.AspNet.Mvc ActionsMatchingRouteAndMethodConstraints = matchingRouteAndMethodConstraints, ActionsMatchingRouteAndMethodAndDynamicConstraints = matchingRouteAndMethodAndDynamicConstraints, - ActionsMatchingWithConstraints = matchesWithConstraints, + FinalMatches = finalMatches, SelectedAction = selectedAction }); } - return selectedAction; + return Task.FromResult(selectedAction); + } + else + { + if (_logger.IsEnabled(TraceType.Information)) + { + _logger.WriteValues(new DefaultActionSelectorSelectAsyncValues() + { + ActionsMatchingRouteConstraints = matchingRouteConstraints, + ActionsMatchingRouteAndMethodConstraints = matchingRouteAndMethodConstraints, + ActionsMatchingRouteAndMethodAndDynamicConstraints = + matchingRouteAndMethodAndDynamicConstraints, + FinalMatches = finalMatches, + }); + } + + var actionNames = string.Join( + Environment.NewLine, + finalMatches.Select(a => a.DisplayName)); + + var message = Resources.FormatDefaultActionSelector_AmbiguousActions( + Environment.NewLine, + actionNames); + + throw new AmbiguousActionException(message); } } } + /// + /// Returns the set of best matching actions. + /// + /// The set of actions that satisfy all constraints. + /// A list of the best matching actions. + protected virtual IReadOnlyList SelectBestActions(IReadOnlyList actions) + { + return actions; + } + private bool MatchMethodConstraints(ActionDescriptor descriptor, RouteContext context) { return descriptor.MethodConstraints == null || @@ -117,76 +149,6 @@ namespace Microsoft.AspNet.Mvc descriptor.DynamicConstraints.All(c => c.Accept(context)); } - protected virtual async Task SelectBestCandidate( - RouteContext context, - List candidates) - { - var applicableCandiates = new List(); - foreach (var action in candidates) - { - var isApplicable = true; - var candidate = new ActionDescriptorCandidate() - { - Action = action, - }; - - var actionContext = new ActionContext(context, action); - var actionBindingContext = await _bindingProvider.GetActionBindingContextAsync(actionContext); - - foreach (var parameter in action.Parameters.Where(p => p.ParameterBindingInfo != null)) - { - if (!ValueProviderResult.CanConvertFromString(parameter.ParameterBindingInfo.ParameterType)) - { - continue; - } - - if (await actionBindingContext.ValueProvider.ContainsPrefixAsync( - parameter.ParameterBindingInfo.Prefix)) - { - candidate.FoundParameters++; - if (parameter.IsOptional) - { - candidate.FoundOptionalParameters++; - } - } - else if (!parameter.IsOptional) - { - isApplicable = false; - break; - } - } - - if (isApplicable) - { - applicableCandiates.Add(candidate); - } - } - - if (applicableCandiates.Count == 0) - { - return null; - } - - var mostParametersSatisfied = - applicableCandiates - .GroupBy(c => c.FoundParameters) - .OrderByDescending(g => g.Key) - .First(); - - var fewestOptionalParameters = - mostParametersSatisfied - .GroupBy(c => c.FoundOptionalParameters) - .OrderBy(g => g.Key).First() - .ToArray(); - - if (fewestOptionalParameters.Length > 1) - { - throw new InvalidOperationException("The actions are ambiguious."); - } - - return fewestOptionalParameters[0].Action; - } - // This method attempts to ensure that the route that's about to generate a link will generate a link // to an existing action. This method is called by a route (through MvcApplication) prior to generating // any link - this gives WebFX a chance to 'veto' the values provided by a route. @@ -222,14 +184,5 @@ namespace Microsoft.AspNet.Mvc return descriptors.Items; } - - private class ActionDescriptorCandidate - { - public ActionDescriptor Action { get; set; } - - public int FoundParameters { get; set; } - - public int FoundOptionalParameters { get; set; } - } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorSelectAsyncValues.cs b/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorSelectAsyncValues.cs index 4699f7c459..a41fca2cb6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorSelectAsyncValues.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Logging/DefaultActionSelectorSelectAsyncValues.cs @@ -38,12 +38,12 @@ namespace Microsoft.AspNet.Mvc.Logging public IReadOnlyList ActionsMatchingRouteAndMethodAndDynamicConstraints { get; set; } /// - /// The actions that matched with at least one constraint. + /// The list of actions that are the best matches. These match all constraints. /// - public IReadOnlyList ActionsMatchingWithConstraints { get; set; } + public IReadOnlyList FinalMatches { get; set; } /// - /// The selected action. + /// The selected action. Will be null if no matches are found or more than one match is found. /// public ActionDescriptor SelectedAction { get; set; } @@ -65,8 +65,8 @@ namespace Microsoft.AspNet.Mvc.Logging builder.Append("\tActions matching route, method, and dynamic constraints: "); StringBuilderHelpers.Append(builder, ActionsMatchingRouteAndMethodAndDynamicConstraints, Formatter); builder.AppendLine(); - builder.Append("\tActions matching with at least one constraint: "); - StringBuilderHelpers.Append(builder, ActionsMatchingWithConstraints, Formatter); + builder.Append("\tBest Matches: "); + StringBuilderHelpers.Append(builder, FinalMatches, Formatter); builder.AppendLine(); builder.Append("\tSelected action: "); builder.Append(Formatter(SelectedAction)); diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index be86cd7f05..72f9f30018 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1482,6 +1482,22 @@ namespace Microsoft.AspNet.Mvc.Core return GetString("AttributeRoute_NullTemplateRepresentation"); } + /// + /// Multiple actions matched. The following actions matched route data and had all constraints satisfied:{0}{0}{1} + /// + internal static string DefaultActionSelector_AmbiguousActions + { + get { return GetString("DefaultActionSelector_AmbiguousActions"); } + } + + /// + /// Multiple actions matched. The following actions matched route data and had all constraints satisfied:{0}{0}{1} + /// + internal static string FormatDefaultActionSelector_AmbiguousActions(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("DefaultActionSelector_AmbiguousActions"), p0, p1); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index dc3b141a1a..4633517ed2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -402,4 +402,8 @@ (none) + + Multiple actions matched. The following actions matched route data and had all constraints satisfied:{0}{0}{1} + 0 is the newline - 1 is a newline separate list of action display names + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionAttributeTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionAttributeTests.cs index 607846e02a..34daccd566 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ActionAttributeTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionAttributeTests.cs @@ -87,26 +87,6 @@ namespace Microsoft.AspNet.Mvc Assert.Equal(null, result); } - [Theory] - [InlineData("GET")] - [InlineData("POST")] - public async Task HttpMethodAttribute_DefaultMethod_IgnoresMethodsWithCustomAttributesAndInvalidMethods(string verb) - { - // Arrange - // Note no action name is passed, hence should return a null action descriptor. - var routeContext = new RouteContext(GetHttpContext(verb)); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "HttpMethodAttributeTests_DefaultMethodValidation" }, - }; - - // Act - var result = await InvokeActionSelector(routeContext); - - // Assert - Assert.Equal("Index", result.Name); - } - [Theory] [InlineData("Put")] [InlineData("RPCMethod")] @@ -198,12 +178,9 @@ namespace Microsoft.AspNet.Mvc var actionCollectionDescriptorProvider = new DefaultActionDescriptorsCollectionProvider(serviceContainer); var decisionTreeProvider = new ActionSelectorDecisionTreeProvider(actionCollectionDescriptorProvider); - var bindingProvider = new Mock(); - var defaultActionSelector = new DefaultActionSelector( actionCollectionDescriptorProvider, decisionTreeProvider, - bindingProvider.Object, NullLoggerFactory.Instance); return await defaultActionSelector.SelectAsync(context); @@ -243,14 +220,15 @@ namespace Microsoft.AspNet.Mvc private class CustomActionConvention : DefaultActionDiscoveryConventions { - public override IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) + public override IEnumerable GetActions([NotNull]MethodInfo methodInfo, [NotNull]TypeInfo controllerTypeInfo) { - if (methodInfo.Name.Equals("PostSomething", StringComparison.OrdinalIgnoreCase)) + var actions = new List(base.GetActions(methodInfo, controllerTypeInfo)); + if (methodInfo.Name == "PostSomething") { - return new[] { "POST" }; + actions[0].HttpMethods = new string[] { "POST" }; } - return null; + return actions; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsActionSelectionTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsActionSelectionTests.cs index 37088e7488..56ed6be735 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsActionSelectionTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsActionSelectionTests.cs @@ -21,16 +21,15 @@ namespace Microsoft.AspNet.Mvc { public class DefaultActionDiscoveryConventionsActionSelectionTests { - [Theory] - [InlineData("GET")] - [InlineData("POST")] - public async Task ActionSelection_IndexSelectedByDefaultInAbsenceOfVerbOnlyMethod(string verb) + [Fact] + public async Task ActionSelection_ActionSelectedByName() { // Arrange - var routeContext = new RouteContext(GetHttpContext(verb)); + var routeContext = new RouteContext(GetHttpContext("GET")); routeContext.RouteData.Values = new Dictionary { - { "controller", "RpcOnly" } + { "controller", "RpcOnly" }, + { "action", "Index" } }; // Act @@ -40,101 +39,7 @@ namespace Microsoft.AspNet.Mvc Assert.Equal("Index", result.Name); } - [Theory] - [InlineData("GET")] - [InlineData("POST")] - public async Task ActionSelection_PrefersVerbOnlyMethodOverIndex(string verb) - { - // Arrange - var routeContext = new RouteContext(GetHttpContext(verb)); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "MixedRpcAndRest" } - }; - - // Act - var result = await InvokeActionSelector(routeContext); - - // Assert - Assert.Equal(verb, result.Name, StringComparer.OrdinalIgnoreCase); - } - - [Theory] - [InlineData("PUT")] - [InlineData("DELETE")] - [InlineData("PATCH")] - public async Task ActionSelection_IndexNotSelectedByDefaultExceptGetAndPostVerbs(string verb) - { - // Arrange - var routeContext = new RouteContext(GetHttpContext(verb)); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "RpcOnly" } - }; - - // Act - var result = await InvokeActionSelector(routeContext); - - // Assert - Assert.Equal(null, result); - } - - [Theory] - [InlineData("HEAD")] - [InlineData("OPTIONS")] - public async Task ActionSelection_NoConventionBasedRoutingForHeadAndOptions(string verb) - { - // Arrange - var routeContext = new RouteContext(GetHttpContext(verb)); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "MixedRpcAndRest" }, - }; - - // Act - var result = await InvokeActionSelector(routeContext); - - // Assert - Assert.Equal(null, result); - } - - [Theory] - [InlineData("HEAD")] - [InlineData("OPTIONS")] - public async Task ActionSelection_ActionNameBasedRoutingForHeadAndOptions(string verb) - { - // Arrange - var routeContext = new RouteContext(GetHttpContext(verb)); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "MixedRpcAndRest" }, - { "action", verb }, - }; - - // Act - var result = await InvokeActionSelector(routeContext); - - // Assert - Assert.Equal(verb, result.Name, StringComparer.OrdinalIgnoreCase); - } - - [Fact] - public async Task ActionSelection_ChangeDefaultConventionPicksCustomMethodForPost_DefaultMethodIsSelectedForGet() - { - // Arrange - var routeContext = new RouteContext(GetHttpContext("GET")); - routeContext.RouteData.Values = new Dictionary - { - { "controller", "RpcOnly" } - }; - - // Act - var result = await InvokeActionSelector(routeContext, new CustomActionConvention()); - - // Assert - Assert.Equal("INDEX", result.Name, StringComparer.OrdinalIgnoreCase); - } - + // Uses custom conventions to map a web-api-style action [Fact] public async Task ActionSelection_ChangeDefaultConventionPicksCustomMethodForPost_CutomMethodIsSelected() { @@ -177,12 +82,9 @@ namespace Microsoft.AspNet.Mvc var actionCollectionDescriptorProvider = new DefaultActionDescriptorsCollectionProvider(serviceContainer); var decisionTreeProvider = new ActionSelectorDecisionTreeProvider(actionCollectionDescriptorProvider); - var bindingProvider = new Mock(); - var defaultActionSelector = new DefaultActionSelector( actionCollectionDescriptorProvider, decisionTreeProvider, - bindingProvider.Object, NullLoggerFactory.Instance); return await defaultActionSelector.SelectAsync(context); @@ -222,64 +124,19 @@ namespace Microsoft.AspNet.Mvc .Contains(typeInfo); } - public override IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) + public override IEnumerable GetActions([NotNull]MethodInfo methodInfo, [NotNull]TypeInfo controllerTypeInfo) { - if (methodInfo.Name.Equals("PostSomething", StringComparison.OrdinalIgnoreCase)) + var actions = new List( + base.GetActions(methodInfo, controllerTypeInfo) ?? + new List()); + + if (methodInfo.Name == "PostSomething") { - return new[] { "POST" }; + actions[0].HttpMethods = new string[] { "POST" }; + actions[0].RequireActionNameMatch = false; } - return null; - } - } - - private class MixedRpcAndRestController - { - public void Index() - { - } - - public void Get() - { - } - - public void Post() - { } - - public void GetSomething() - { } - - // This will be treated as an RPC method. - public void Head() - { - } - - // This will be treated as an RPC method. - public void Options() - { - } - } - - private class RestOnlyController - { - public void Get() - { - } - - public void Put() - { - } - - public void Post() - { - } - - public void Delete() - { - } - - public void Patch() - { + return actions; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs index d0fef35e57..2fa285e787 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs @@ -54,7 +54,7 @@ namespace Microsoft.AspNet.Mvc Assert.Empty(values.ActionsMatchingRouteConstraints); Assert.Empty(values.ActionsMatchingRouteAndMethodConstraints); Assert.Empty(values.ActionsMatchingRouteAndMethodAndDynamicConstraints); - Assert.Empty(values.ActionsMatchingWithConstraints); + Assert.Empty(values.FinalMatches); Assert.Null(values.SelectedAction); } @@ -106,12 +106,60 @@ namespace Microsoft.AspNet.Mvc Assert.Equal("DefaultActionSelector.SelectAsync", write.Scope); var values = Assert.IsType(write.State); Assert.Equal("DefaultActionSelector.SelectAsync", values.Name); - Assert.NotEmpty(values.ActionsMatchingRouteConstraints); - Assert.NotEmpty(values.ActionsMatchingRouteAndMethodConstraints); - Assert.NotEmpty(values.ActionsMatchingWithConstraints); + Assert.Equal(actions, values.ActionsMatchingRouteConstraints); + Assert.Equal(actions, values.ActionsMatchingRouteAndMethodConstraints); + Assert.Equal(matched, Assert.Single(values.FinalMatches)); Assert.Equal(matched, values.SelectedAction); } + [Fact] + public async void SelectAsync_AmbiguousActions_LogIsCorrect() + { + // Arrange + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink); + + var actions = new ActionDescriptor[] + { + new ActionDescriptor() { DisplayName = "A1" }, + new ActionDescriptor() { DisplayName = "A2" }, + }; + + var selector = CreateSelector(actions, loggerFactory); + + var routeContext = CreateRouteContext("POST"); + + // Act + await Assert.ThrowsAsync(async () => + { + await selector.SelectAsync(routeContext); + }); + + // Assert + Assert.Equal(1, sink.Scopes.Count); + var scope = sink.Scopes[0]; + Assert.Equal(typeof(DefaultActionSelector).FullName, scope.LoggerName); + Assert.Equal("DefaultActionSelector.SelectAsync", scope.Scope); + + // There is a record for IsEnabled and one for WriteCore. + Assert.Equal(2, sink.Writes.Count); + + var enabled = sink.Writes[0]; + Assert.Equal(typeof(DefaultActionSelector).FullName, enabled.LoggerName); + Assert.Equal("DefaultActionSelector.SelectAsync", enabled.Scope); + Assert.Null(enabled.State); + + var write = sink.Writes[1]; + Assert.Equal(typeof(DefaultActionSelector).FullName, write.LoggerName); + Assert.Equal("DefaultActionSelector.SelectAsync", write.Scope); + var values = Assert.IsType(write.State); + Assert.Equal("DefaultActionSelector.SelectAsync", values.Name); + Assert.Equal(actions, values.ActionsMatchingRouteConstraints); + Assert.Equal(actions, values.ActionsMatchingRouteAndMethodConstraints); + Assert.Equal(actions, values.FinalMatches); + Assert.Null(values.SelectedAction); + } + [Fact] public void HasValidAction_Match() { @@ -262,6 +310,43 @@ namespace Microsoft.AspNet.Mvc Assert.Null(action); } + [Fact] + public async Task SelectAsync_Ambiguous() + { + // Arrange + var expectedMessage = + "Multiple actions matched. " + + "The following actions matched route data and had all constraints satisfied:" + Environment.NewLine + + Environment.NewLine + + "Ambiguous1" + Environment.NewLine + + "Ambiguous2"; + + var actions = new ActionDescriptor[] + { + CreateAction(area: null, controller: "Store", action: "Buy"), + CreateAction(area: null, controller: "Store", action: "Buy"), + CreateAction(area: null, controller: "Store", action: "Cart"), + }; + + actions[0].DisplayName = "Ambiguous1"; + actions[1].DisplayName = "Ambiguous2"; + + var selector = CreateSelector(actions); + var context = CreateRouteContext("GET"); + + context.RouteData.Values.Add("controller", "Store"); + context.RouteData.Values.Add("action", "Buy"); + + // Act + var ex = await Assert.ThrowsAsync(async () => + { + await selector.SelectAsync(context); + }); + + // Assert + Assert.Equal(expectedMessage, ex.Message); + } + private static ActionDescriptor[] GetActions() { return new ActionDescriptor[] @@ -304,12 +389,7 @@ namespace Microsoft.AspNet.Mvc var decisionTreeProvider = new ActionSelectorDecisionTreeProvider(actionProvider.Object); - var bindingProvider = new Mock(MockBehavior.Strict); - bindingProvider - .Setup(bp => bp.GetActionBindingContextAsync(It.IsAny())) - .Returns(Task.FromResult(null)); - - return new DefaultActionSelector(actionProvider.Object, decisionTreeProvider, bindingProvider.Object, loggerFactory); + return new DefaultActionSelector(actionProvider.Object, decisionTreeProvider, loggerFactory); } private static VirtualPathContext CreateContext(object routeValues)