diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index aeebe4fcaa..497179801b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Mvc { private DynamicViewData _viewBag; + [NonAction] public void Initialize(IActionResultHelper actionResultHelper) { Result = actionResultHelper; @@ -42,6 +43,7 @@ namespace Microsoft.AspNet.Mvc public IActionResultHelper Result { get; private set; } public IUrlHelper Url { get; set; } + public IPrincipal User { get @@ -70,22 +72,26 @@ namespace Microsoft.AspNet.Mvc } } + [NonAction] public ViewResult View() { return View(view: null); } + [NonAction] public ViewResult View(string view) { return View(view, model: null); } + [NonAction] // TODO #110: May need here and in the overload below. public ViewResult View(object model) { return View(view: null, model: model); } + [NonAction] public ViewResult View(string view, object model) { // Do not override ViewData.Model unless passed a non-null value. @@ -97,26 +103,31 @@ namespace Microsoft.AspNet.Mvc return Result.View(view, ViewData); } + [NonAction] public ContentResult Content(string content) { return Content(content, contentType: null); } + [NonAction] public ContentResult Content(string content, string contentType) { return Content(content, contentType, contentEncoding: null); } + [NonAction] public ContentResult Content(string content, string contentType, Encoding contentEncoding) { return Result.Content(content, contentType, contentEncoding); } + [NonAction] public JsonResult Json(object value) { return Result.Json(value); } + [NonAction] public virtual RedirectResult Redirect(string url) { if (string.IsNullOrEmpty(url)) @@ -127,6 +138,7 @@ namespace Microsoft.AspNet.Mvc return new RedirectResult(url); } + [NonAction] public virtual RedirectResult RedirectPermanent(string url) { if (string.IsNullOrEmpty(url)) @@ -137,21 +149,25 @@ namespace Microsoft.AspNet.Mvc return new RedirectResult(url, permanent: true); } + [NonAction] public RedirectToActionResult RedirectToAction(string actionName) { return RedirectToAction(actionName, routeValues: null); } + [NonAction] public RedirectToActionResult RedirectToAction(string actionName, object routeValues) { return RedirectToAction(actionName, controllerName: null, routeValues: routeValues); } + [NonAction] public RedirectToActionResult RedirectToAction(string actionName, string controllerName) { return RedirectToAction(actionName, controllerName, routeValues: null); } + [NonAction] public RedirectToActionResult RedirectToAction(string actionName, string controllerName, object routeValues) { @@ -159,21 +175,25 @@ namespace Microsoft.AspNet.Mvc TypeHelper.ObjectToDictionary(routeValues)); } + [NonAction] public RedirectToActionResult RedirectToActionPermanent(string actionName) { return RedirectToActionPermanent(actionName, routeValues: null); } + [NonAction] public RedirectToActionResult RedirectToActionPermanent(string actionName, object routeValues) { return RedirectToActionPermanent(actionName, controllerName: null, routeValues: routeValues); } + [NonAction] public RedirectToActionResult RedirectToActionPermanent(string actionName, string controllerName) { return RedirectToActionPermanent(actionName, controllerName, routeValues: null); } + [NonAction] public RedirectToActionResult RedirectToActionPermanent(string actionName, string controllerName, object routeValues) { @@ -181,46 +201,55 @@ namespace Microsoft.AspNet.Mvc TypeHelper.ObjectToDictionary(routeValues), permanent: true); } + [NonAction] public RedirectToRouteResult RedirectToRoute(string routeName) { return RedirectToRoute(routeName, routeValues: null); } + [NonAction] public RedirectToRouteResult RedirectToRoute(object routeValues) { return RedirectToRoute(routeName: null, routeValues: routeValues); } + [NonAction] public RedirectToRouteResult RedirectToRoute(string routeName, object routeValues) { return new RedirectToRouteResult(Url, routeName, routeValues); } + [NonAction] public RedirectToRouteResult RedirectToRoutePermanent(string routeName) { return RedirectToRoutePermanent(routeName, routeValues: null); } + [NonAction] public RedirectToRouteResult RedirectToRoutePermanent(object routeValues) { return RedirectToRoutePermanent(routeName: null, routeValues: routeValues); } + [NonAction] public RedirectToRouteResult RedirectToRoutePermanent(string routeName, object routeValues) { return new RedirectToRouteResult(Url, routeName, routeValues, permanent: true); } + [NonAction] public virtual void OnActionExecuting([NotNull] ActionExecutingContext context) { } + [NonAction] public virtual void OnActionExecuted([NotNull] ActionExecutedContext context) { } + [NonAction] public virtual async Task OnActionExecutionAsync( - [NotNull] ActionExecutingContext context, + [NotNull] ActionExecutingContext context, [NotNull] ActionExecutionDelegate next) { OnActionExecuting(context); diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs index 10aa619073..1017ff50a0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs @@ -90,7 +90,10 @@ namespace Microsoft.AspNet.Mvc // The SpecialName bit is set to flag members that are treated in a special way by some compilers // (such as property accessors and operator overloading methods). !method.IsSpecialName && - !method.IsDefined(typeof(NonActionAttribute)); + !method.IsDefined(typeof(NonActionAttribute)) && + + // Overriden methods from Object class, e.g. Equals(Object), GetHashCode(), etc., are not valid. + method.GetBaseDefinition().DeclaringType != typeof(object); } public virtual IEnumerable GetSupportedHttpMethods(MethodInfo methodInfo) diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index b77a7cadb2..a4a4d62fed 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -53,6 +53,12 @@ namespace Microsoft.AspNet.Mvc .Select(t => _controllerDescriptorFactory.CreateControllerDescriptor(t)) .ToArray(); + return GetDescriptors(controllerDescriptors); + } + + // Internal for unit testing. + internal IEnumerable GetDescriptors(IEnumerable controllerDescriptors) + { foreach (var cd in controllerDescriptors) { var controllerAttributes = cd.ControllerTypeInfo.GetCustomAttributes(inherit: true).ToArray(); @@ -63,7 +69,7 @@ namespace Microsoft.AspNet.Mvc .OrderBy(d => d, FilterDescriptorOrderComparer.Comparer) .ToArray(); - foreach (var methodInfo in cd.ControllerTypeInfo.DeclaredMethods) + foreach (var methodInfo in cd.ControllerTypeInfo.AsType().GetMethods()) { var actionInfos = _conventions.GetActions(methodInfo, cd.ControllerTypeInfo); if (actionInfos == null) diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs index 16748dd250..af9ab3ca0a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs @@ -2,8 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Linq; +using System.Reflection; using Microsoft.AspNet.Mvc.ModelBinding; -using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Testing; using Xunit; @@ -12,6 +13,17 @@ namespace Microsoft.AspNet.Mvc.Core { public class ControllerTests { + public static IEnumerable PublicNormalMethodsFromController + { + get + { + return typeof(Controller).GetTypeInfo() + .DeclaredMethods + .Where(method => method.IsPublic && !method.IsSpecialName) + .Select(method => new [] { method }); + } + } + [Fact] public void SettingViewData_AlsoUpdatesViewBag() { @@ -248,6 +260,14 @@ namespace Microsoft.AspNet.Mvc.Core Assert.Equal(TypeHelper.ObjectToDictionary(routeValues), resultPermanent.RouteValues); } + [Theory] + [MemberData("PublicNormalMethodsFromController")] + public void NonActionAttribute_IsOnEveryPublicNormalMethodFromController(MethodInfo method) + { + // Arrange & Act & Assert + Assert.True(method.IsDefined(typeof(NonActionAttribute))); + } + public static IEnumerable RedirectTestData { get diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj index f651e32649..52dc1db032 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj @@ -49,6 +49,7 @@ + @@ -60,4 +61,4 @@ - + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs new file mode 100644 index 0000000000..5ab6c62544 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs @@ -0,0 +1,180 @@ +// 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.Collections.Generic; +using System.Linq; +using System.Reflection; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class ReflectedActionDescriptorProviderTests + { + private DefaultActionDiscoveryConventions _actionDiscoveryConventions = + new DefaultActionDiscoveryConventions(); + private IControllerDescriptorFactory _controllerDescriptorFactory = new DefaultControllerDescriptorFactory(); + private IParameterDescriptorFactory _parameterDescriptorFactory = new DefaultParameterDescriptorFactory(); + + [Fact] + public void GetDescriptors_GetsDescriptorsOnlyForValidActionsInBaseAndDerivedController() + { + // Arrange & Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + // "NewMethod" is a public method declared with keyword "new". + Assert.Equal(new[] { "GetFromDerived", "NewMethod", "GetFromBase" }, actionNames); + } + + [Fact] + public void GetDescriptors_Ignores_OverridenRedirect_FromControllerClass() + { + // Arrange & Act + var actionNames = GetDescriptors(typeof(BaseController).GetTypeInfo()).Select(a => a.Name); + + // Assert + Assert.False(actionNames.Contains("Redirect")); + } + + [Fact] + public void GetDescriptors_Ignores_PrivateMethod_FromUserDefinedController() + { + // Arrange & Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + Assert.False(actionNames.Contains("PrivateMethod")); + } + + [Fact] + public void GetDescriptors_Ignores_Constructor_FromUserDefinedController() + { + // Arrange & Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + Assert.False(actionNames.Contains("DerivedController")); + } + + [Fact] + public void GetDescriptors_Ignores_OperatorOverloadingMethod_FromOperatorOverloadingController() + { + // Arrange & Act + var actionDescriptors = GetDescriptors(typeof(OperatorOverloadingController).GetTypeInfo()); + + // Assert + Assert.Empty(actionDescriptors); + } + + [Fact] + public void GetDescriptors_Ignores_GenericMethod_FromUserDefinedController() + { + // Arrange & Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + Assert.False(actionNames.Contains("GenericMethod")); + } + + [Fact] + public void GetDescriptors_Ignores_OverridenNonActionMethod_FromDerivedController() + { + // Arrange & Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + Assert.False(actionNames.Contains("OverridenNonActionMethod")); + } + + [Fact] + public void GetDescriptors_Ignores_MethodsFromObjectClass_FromUserDefinedController() + { + // Arrange + var methodsFromObjectClass = typeof(object).GetMethods().Select(m => m.Name); + + // Act + var actionNames = GetActionNamesFromDerivedController(); + + // Assert + Assert.Empty(methodsFromObjectClass.Intersect(actionNames)); + } + + private IEnumerable GetActionNamesFromDerivedController() + { + return GetDescriptors(typeof(DerivedController).GetTypeInfo()).Select(a => a.Name); + } + + private IEnumerable GetDescriptors(TypeInfo controllerTypeInfo) + { + var provider = new ReflectedActionDescriptorProvider(null, + _actionDiscoveryConventions, + _controllerDescriptorFactory, + _parameterDescriptorFactory, + null); + var testControllers = new TypeInfo[] + { + controllerTypeInfo, + }; + var controllerDescriptors = testControllers + .Select(t => _controllerDescriptorFactory.CreateControllerDescriptor(t)); + return provider.GetDescriptors(controllerDescriptors); + } + + private class DerivedController : BaseController + { + public void GetFromDerived() // Valid action method. + { + } + + [HttpGet] + public override void OverridenNonActionMethod() + { + } + + public new void NewMethod() // Valid action method. + { + } + + public void GenericMethod() + { + } + + private void PrivateMethod() + { + } + } + + private class OperatorOverloadingController : Controller + { + public static OperatorOverloadingController operator +( + OperatorOverloadingController c1, + OperatorOverloadingController c2) + { + return new OperatorOverloadingController(); + } + } + + private class BaseController : Controller + { + public void GetFromBase() // Valid action method. + { + } + + [NonAction] + public virtual void OverridenNonActionMethod() + { + } + + [NonAction] + public virtual void NewMethod() + { + } + + public override RedirectResult Redirect(string url) + { + return base.Redirect(url + "#RedirectOverride"); + } + } + } +}