diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs b/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs index 859f973e5d..13ac206c54 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs @@ -1,4 +1,6 @@ -namespace Microsoft.AspNet.Mvc +using System.Collections.Generic; + +namespace Microsoft.AspNet.Mvc { public class ActionInfo { diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs index 6fda17d69e..ccdf85ff6e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs @@ -1,28 +1,34 @@ using System; -using System.Collections; using System.Collections.Generic; +using System.Linq; using System.Reflection; namespace Microsoft.AspNet.Mvc { public class DefaultActionDiscoveryConventions : IActionDiscoveryConventions { - private static readonly string[] _supportedHttpMethodsByConvention = - { - "GET", - "POST", - "PUT", - "DELETE", + private static readonly string[] _supportedHttpMethodsByConvention = + { + "GET", + "POST", + "PUT", + "DELETE", "PATCH", }; - public virtual bool IsController(TypeInfo typeInfo) + private static readonly string[] _supportedHttpMethodsForDefaultMethod = { - if (typeInfo == null) - { - throw new ArgumentNullException("typeInfo"); - } + "GET", + "POST" + }; + public virtual string DefaultMethodName + { + get { return "Index"; } + } + + public virtual bool IsController([NotNull] TypeInfo typeInfo) + { if (!typeInfo.IsClass || typeInfo.IsAbstract || typeInfo.ContainsGenericParameters) @@ -37,46 +43,82 @@ namespace Microsoft.AspNet.Mvc return typeInfo.Name.EndsWith("Controller", StringComparison.OrdinalIgnoreCase) || typeof(Controller).GetTypeInfo().IsAssignableFrom(typeInfo); - } - public IEnumerable GetActions(MethodInfo methodInfo) + // If the convention is All methods starting with Get do not have an action name, + // for a input GetXYZ methodInfo, the return value will be + // { { HttpMethods = "GET", ActionName = "GetXYZ", RequireActionNameMatch = false }} + public virtual IEnumerable GetActions( + [NotNull] MethodInfo methodInfo, + [NotNull] TypeInfo controllerTypeInfo) { - if (methodInfo == null) - { - throw new ArgumentNullException("methodInfo"); - } - if (!IsValidMethod(methodInfo)) { return null; } - for (var i = 0; i < _supportedHttpMethodsByConvention.Length; i++) + var actionInfos = new List(); + var httpMethods = GetSupportedHttpMethods(methodInfo); + if (httpMethods != null && httpMethods.Any()) { - if (methodInfo.Name.Equals(_supportedHttpMethodsByConvention[i], StringComparison.OrdinalIgnoreCase)) - { - return new [] { + return new[] { new ActionInfo() { - HttpMethods = new[] { _supportedHttpMethodsByConvention[i] }, + 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 (IsDefaultMethod(methodInfo)) + { + var existingHttpMethods = new HashSet(); + foreach (var validMethodName in controllerTypeInfo.DeclaredMethods) + { + if (!IsValidMethod(validMethodName)) + { + continue; + } + + var methodNames = GetSupportedHttpMethods(validMethodName); + if (methodNames != null ) + { + existingHttpMethods.UnionWith(methodNames); + } + } + + var undefinedHttpMethods = _supportedHttpMethodsForDefaultMethod.Except( + existingHttpMethods, + StringComparer.Ordinal) + .ToArray(); + if (undefinedHttpMethods.Any()) + { + actionInfos.Add(new ActionInfo() + { + HttpMethods = undefinedHttpMethods, + ActionName = methodInfo.Name, + RequireActionNameMatch = false, + }); } } - // TODO: Consider mapping Index here to both Get and also to Index - - return new[] - { + actionInfos.Add( new ActionInfo() { ActionName = methodInfo.Name, RequireActionNameMatch = true, - } - }; + }); + + return actionInfos; + } + + public virtual bool IsDefaultMethod([NotNull] MethodInfo methodInfo) + { + return String.Equals(methodInfo.Name, DefaultMethodName, StringComparison.OrdinalIgnoreCase); } public virtual bool IsValidMethod(MethodInfo method) @@ -88,5 +130,17 @@ namespace Microsoft.AspNet.Mvc !method.IsGenericMethod && !method.IsSpecialName; } + + public virtual IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) + { + var ret = + _supportedHttpMethodsByConvention.FirstOrDefault( + t => methodInfo.Name.Equals(t, StringComparison.OrdinalIgnoreCase)); + + if (ret != null) + { + yield return ret; + } + } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/IActionDiscoveryConventions.cs b/src/Microsoft.AspNet.Mvc.Core/IActionDiscoveryConventions.cs index 9a5c0bf6b3..e85f9eb9e0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IActionDiscoveryConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IActionDiscoveryConventions.cs @@ -7,6 +7,6 @@ namespace Microsoft.AspNet.Mvc { bool IsController(TypeInfo typeInfo); - IEnumerable GetActions(MethodInfo methodInfo); + IEnumerable GetActions(MethodInfo methodInfo, TypeInfo controllerTypeInfo); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index 7b7d04d1d4..0e25e2baf8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -60,8 +60,7 @@ namespace Microsoft.AspNet.Mvc foreach (var methodInfo in cd.ControllerTypeInfo.DeclaredMethods) { - var actionInfos = _conventions.GetActions(methodInfo); - + var actionInfos = _conventions.GetActions(methodInfo, cd.ControllerTypeInfo); if (actionInfos == null) { continue; @@ -123,4 +122,4 @@ namespace Microsoft.AspNet.Mvc return ad; } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ActionSelectionConventionTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ActionSelectionConventionTests.cs new file mode 100644 index 0000000000..05df84c664 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ActionSelectionConventionTests.cs @@ -0,0 +1,304 @@ +#if NET45 + +using Microsoft.AspNet.Abstractions; +using Microsoft.AspNet.DependencyInjection.NestedProviders; +using Moq; +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core.Test +{ + public class ActionSelectionConventionTests + { + private IActionDiscoveryConventions _actionDiscoveryConventions = new DefaultActionDiscoveryConventions(); + private IControllerDescriptorFactory _controllerDescriptorFactory = new DefaultControllerDescriptorFactory(); + private IParameterDescriptorFactory _parameterDescriptorFactory = new DefaultParameterDescriptorFactory(); + private IEnumerable _controllerAssemblies = new[] { Assembly.GetExecutingAssembly() }; + + [Theory] + [InlineData("GET")] + [InlineData("POST")] + public async Task ActionSelection_IndexSelectedByDefaultInAbsenceOfVerbOnlyMethod(string verb) + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext(verb), + new Dictionary + { + { "controller", "RpcOnly" } + }); + + // Act + var result = await InvokeActionSelector(requestContext); + + // Assert + Assert.Equal("Index", result.Name); + } + + [Theory] + [InlineData("GET")] + [InlineData("POST")] + public async Task ActionSelection_PrefersVerbOnlyMethodOverIndex(string verb) + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext(verb), + new Dictionary + { + { "controller", "MixedRpcAndRest" } + }); + + // Act + var result = await InvokeActionSelector(requestContext); + + // Assert + Assert.Equal(verb, result.Name, StringComparer.OrdinalIgnoreCase); + } + + [Theory] + [InlineData("PUT")] + [InlineData("DELETE")] + [InlineData("PATCH")] + public async Task ActionSelection_IndexNotSelectedByDefaultExceptGetAndPostVerbs(string verb) + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext(verb), + new Dictionary + { + { "controller", "RpcOnly" } + }); + + // Act + var result = await InvokeActionSelector(requestContext); + + // Assert + Assert.Equal(null, result); + } + + [Theory] + [InlineData("HEAD")] + [InlineData("OPTIONS")] + public async Task ActionSelection_NoConventionBasedRoutingForHeadAndOptions(string verb) + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext(verb), + new Dictionary + { + {"controller", "MixedRpcAndRest"}, + }); + + // Act + var result = await InvokeActionSelector(requestContext); + + // Assert + Assert.Equal(null, result); + } + + [Theory] + [InlineData("HEAD")] + [InlineData("OPTIONS")] + public async Task ActionSelection_ActionNameBasedRoutingForHeadAndOptions(string verb) + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext(verb), + new Dictionary + { + { "controller", "MixedRpcAndRest" }, + { "action", verb }, + }); + + // Act + var result = await InvokeActionSelector(requestContext); + + // Assert + Assert.Equal(verb, result.Name, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public async Task ActionSelection_ChangeDefaultConventionPicksCustomMethodForPost_DefaultMethodIsSelectedForGet() + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext("GET"), + new Dictionary + { + { "controller", "RpcOnly" } + }); + + // Act + var result = await InvokeActionSelector(requestContext, new CustomActionConvention()); + + // Assert + Assert.Equal("INDEX", result.Name, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public async Task ActionSelection_ChangeDefaultConventionPicksCustomMethodForPost_CutomMethodIsSelected() + { + // Arrange + var requestContext = new RequestContext( + GetHttpContext("POST"), + new Dictionary + { + { "controller", "RpcOnly" } + }); + + // Act + var result = await InvokeActionSelector(requestContext, new CustomActionConvention()); + + // Assert + Assert.Equal("PostSomething", result.Name); + } + + private async Task InvokeActionSelector(RequestContext context) + { + return await InvokeActionSelector(context, _actionDiscoveryConventions); + } + + private async Task InvokeActionSelector(RequestContext context, IActionDiscoveryConventions actionDiscoveryConventions) + { + var actionDescriptorProvider = GetActionDescriptorProvider(actionDiscoveryConventions); + var descriptorProvider = + new NestedProviderManager(new[] { actionDescriptorProvider }); + var bindingProvider = new Mock(); + + var defaultActionSelector = new DefaultActionSelector(descriptorProvider, bindingProvider.Object); + return await defaultActionSelector.SelectAsync(context); + } + + private ReflectedActionDescriptorProvider GetActionDescriptorProvider(IActionDiscoveryConventions actionDiscoveryConventions) + { + var controllerAssemblyProvider = new Mock(); + controllerAssemblyProvider.SetupGet(x => x.CandidateAssemblies).Returns(_controllerAssemblies); + return new ReflectedActionDescriptorProvider( + controllerAssemblyProvider.Object, + actionDiscoveryConventions, + _controllerDescriptorFactory, + _parameterDescriptorFactory, + null); + } + + private static HttpContext GetHttpContext(string httpMethod) + { + var request = new Mock(); + var headers = new Mock(); + request.SetupGet(r => r.Headers).Returns(headers.Object); + request.SetupGet(x => x.Method).Returns(httpMethod); + + var httpContext = new Mock(); + httpContext.SetupGet(c => c.Request).Returns(request.Object); + return httpContext.Object; + } + + private class CustomActionConvention : DefaultActionDiscoveryConventions + { + public override IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) + { + if (methodInfo.Name.Equals("PostSomething", StringComparison.OrdinalIgnoreCase)) + { + return new[] { "POST" }; + } + + return null; + } + } + + #region Controller Classes + + 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() + { + } + } + + private class RpcOnlyController + { + public void Index() + { + } + + public void GetSomething() + { + } + + public void PutSomething() + { + } + + public void PostSomething() + { + } + + public void DeleteSomething() + { + } + + public void PatchSomething() + { + } + } + + private class AmbiguousController + { + public void Index(int i) + { } + + public void Index(string s) + { } + } + + #endregion Controller Classes + } +} + +#endif \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/project.json b/test/Microsoft.AspNet.Mvc.Core.Test/project.json index 3acb6991c7..9f4b126f67 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/project.json +++ b/test/Microsoft.AspNet.Mvc.Core.Test/project.json @@ -6,6 +6,9 @@ "Microsoft.AspNet.Testing": "0.1-alpha-*", "Microsoft.AspNet.Mvc.ModelBinding": "", "Microsoft.AspNet.Mvc.Rendering": "", + "Microsoft.AspNet.DependencyInjection" : "0.1-alpha-*", + "Microsoft.AspNet.Abstractions" : "0.1-alpha-*", + "Microsoft.AspNet.Mvc.ModelBinding" : "", "Moq": "4.2.1312.1622", "Xunit.KRunner": "0.1-alpha-*", "xunit.abstractions": "2.0.0-aspnet-*",