diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerAttribute.cs new file mode 100644 index 0000000000..2e225dac6d --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerAttribute.cs @@ -0,0 +1,17 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc +{ + /// + /// Indicates that the type and any derived types that this attribute is applied to + /// are considered a controller by the default controller discovery mechanism, unless + /// is applied to any type in the hierarchy. + /// + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] + public class ControllerAttribute : Attribute + { + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs index eee27e65f9..fd30b08024 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs @@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// A base class for an MVC controller without view support. /// + [Controller] public abstract class ControllerBase { private ControllerContext _controllerContext; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerTypeProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerTypeProvider.cs index e8135f9cef..a6d93d60c6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerTypeProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerTypeProvider.cs @@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers { var candidateAssemblies = new HashSet(_assemblyProvider.CandidateAssemblies); var types = candidateAssemblies.SelectMany(a => a.DefinedTypes); - return types.Where(typeInfo => IsController(typeInfo, candidateAssemblies)); + return types.Where(typeInfo => IsController(typeInfo)); } } @@ -44,79 +44,48 @@ namespace Microsoft.AspNetCore.Mvc.Controllers /// Returns true if the is a controller. Otherwise false. /// /// The . - /// The set of candidate assemblies. /// true if the is a controller. Otherwise false. - protected internal virtual bool IsController( - TypeInfo typeInfo, - ISet candidateAssemblies) + protected internal virtual bool IsController(TypeInfo typeInfo) { if (typeInfo == null) { throw new ArgumentNullException(nameof(typeInfo)); } - if (candidateAssemblies == null) - { - throw new ArgumentNullException(nameof(candidateAssemblies)); - } - if (!typeInfo.IsClass) { return false; } + if (typeInfo.IsAbstract) { return false; } + // We only consider public top-level classes as controllers. IsPublic returns false for nested // classes, regardless of visibility modifiers if (!typeInfo.IsPublic) { return false; } + if (typeInfo.ContainsGenericParameters) { return false; } - if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) && - !DerivesFromController(typeInfo, candidateAssemblies)) + + if (typeInfo.IsDefined(typeof(NonControllerAttribute))) { return false; } - if (typeInfo.IsDefined(typeof(NonControllerAttribute))) + + if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) && + !typeInfo.IsDefined(typeof(ControllerAttribute))) { return false; } return true; } - - private bool DerivesFromController(TypeInfo typeInfo, ISet candidateAssemblies) - { - while (typeInfo != ObjectTypeInfo) - { - var baseTypeInfo = typeInfo.BaseType.GetTypeInfo(); - - // A base type will be treated as a controller if - // a) it ends in the term "Controller" and - // b) it's assembly is one of the candidate assemblies we're considering. This ensures that the assembly - // the base type is declared in references Mvc. - if (baseTypeInfo.Name.EndsWith(ControllerTypeName, StringComparison.Ordinal) && - candidateAssemblies.Contains(baseTypeInfo.Assembly)) - { - return true; - } - - // c). The base type is called 'Controller. - if (string.Equals(baseTypeInfo.Name, ControllerTypeName, StringComparison.Ordinal)) - { - return true; - } - - typeInfo = baseTypeInfo; - } - - return false; - } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNetCore.Mvc.WebApiCompatShim/ApiController.cs index 67caebc099..f89692de8f 100644 --- a/src/Microsoft.AspNetCore.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNetCore.Mvc.WebApiCompatShim/ApiController.cs @@ -21,6 +21,7 @@ namespace System.Web.Http [UseWebApiActionConventions] [UseWebApiParameterConventions] [UseWebApiOverloading] + [Controller] public abstract class ApiController : IDisposable { private ControllerContext _controllerContext; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerTypeProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerTypeProviderTest.cs index deeb7fdadb..0a0df2d162 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerTypeProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerTypeProviderTest.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -53,10 +53,10 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert - Assert.False(isController); + Assert.True(isController); } [Fact] @@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -81,7 +81,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -109,7 +109,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -123,7 +123,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -137,7 +137,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -151,7 +151,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.False(isController); @@ -179,7 +179,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -193,7 +193,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -207,7 +207,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -221,7 +221,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(typeInfo, CandidateAssemblies); + var isController = provider.IsController(typeInfo); // Assert Assert.True(isController); @@ -230,18 +230,31 @@ namespace Microsoft.AspNetCore.Mvc.Controllers [Theory] [InlineData(typeof(DescendantLevel1))] [InlineData(typeof(DescendantLevel2))] - public void IsController_ReturnsTrue_IfAncestorTypeNameHasControllerSuffix(Type type) + public void IsController_ReturnsTrue_IfAncestorTypeHasControllerAttribute(Type type) { // Arrange var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies); + var isController = provider.IsController(type.GetTypeInfo()); // Assert Assert.True(isController); } + [Fact] + public void IsController_ReturnsFalse_IfAncestorTypeDoesNotHaveControllerAttribute() + { + // Arrange + var provider = GetControllerTypeProvider(); + + // Act + var isController = provider.IsController(typeof(NoSuffixNoControllerAttribute).GetTypeInfo()); + + // Assert + Assert.False(isController); + } + [Theory] [InlineData(typeof(BaseNonControllerController))] [InlineData(typeof(BaseNonControllerControllerChild))] @@ -249,6 +262,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers [InlineData(typeof(BasePocoNonControllerControllerChild))] [InlineData(typeof(NonController))] [InlineData(typeof(NonControllerChild))] + [InlineData(typeof(BaseNonControllerAttributeChildControllerControllerAttributeController))] [InlineData(typeof(PersonModel))] // Verifies that POCO type hierarchies that don't derive from controller return false. public void IsController_ReturnsFalse_IfTypeOrAncestorHasNonControllerAttribute(Type type) { @@ -256,7 +270,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers var provider = GetControllerTypeProvider(); // Act - var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies); + var isController = provider.IsController(type.GetTypeInfo()); // Assert Assert.False(isController); @@ -297,11 +311,19 @@ namespace Microsoft.AspNetCore.Mvc.DefaultControllerTypeProviderControllers { } - + [Controller] public abstract class Controller { } + public abstract class NoControllerAttributeBaseController + { + } + + public class NoSuffixNoControllerAttribute : NoControllerAttributeBaseController + { + } + public class OpenGenericController : Controller { } @@ -327,17 +349,19 @@ namespace Microsoft.AspNetCore.Mvc.DefaultControllerTypeProviderControllers { } - public class CustomBaseController + [Controller] + public class CustomBase { } + [Controller] public abstract class CustomAbstractBaseController { } - public class DescendantLevel1 : CustomBaseController + public class DescendantLevel1 : CustomBase { } @@ -369,6 +393,12 @@ namespace Microsoft.AspNetCore.Mvc.DefaultControllerTypeProviderControllers } + [Controller] + public class BaseNonControllerAttributeChildControllerControllerAttributeController : BaseNonControllerController + { + + } + public class BaseNonControllerControllerChild : BaseNonControllerController { diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs index 516031ea03..3dafb33608 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs @@ -331,16 +331,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("This is a basic website.", responseData); } - [Fact] - public async Task TypesWithoutControllerSuffix_DerivingFromTypesWithControllerSuffix_CanBeAccessed() - { - // Act - var response = await Client.GetStringAsync("appointments"); - - // Assert - Assert.Equal("2 appointments available.", response); - } - [Fact] public async Task TypesMarkedAsNonAction_AreInaccessible() { diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ControllerFromServicesTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ControllerFromServicesTests.cs index 58f349d774..fcc926097b 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ControllerFromServicesTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ControllerFromServicesTests.cs @@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task TypesDerivingFromControllerPrefixedTypesAreRegistered() + public async Task TypesDerivingFromTypesWithControllerAttributeAreRegistered() { // Arrange var expected = "4"; diff --git a/test/Microsoft.AspNetCore.Mvc.WebApiCompatShimTest/ApiControllerActionDiscoveryTest.cs b/test/Microsoft.AspNetCore.Mvc.WebApiCompatShimTest/ApiControllerActionDiscoveryTest.cs index 48dec2296a..e66d33ad7a 100644 --- a/test/Microsoft.AspNetCore.Mvc.WebApiCompatShimTest/ApiControllerActionDiscoveryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.WebApiCompatShimTest/ApiControllerActionDiscoveryTest.cs @@ -55,9 +55,9 @@ namespace System.Web.Http // Assert var controllerType = typeof(TestControllers.Blog).GetTypeInfo(); - var actions = results.Where(ad => ad.ControllerTypeInfo == controllerType).ToArray(); + var actions = results.Where(ad => ad.ControllerTypeInfo == controllerType); - Assert.Empty(actions); + Assert.Equal(2, actions.Count()); } [Fact] diff --git a/test/WebSites/BasicWebSite/Controllers/ApplicationBaseController.cs b/test/WebSites/BasicWebSite/Controllers/ApplicationBaseController.cs deleted file mode 100644 index c1e33e2e18..0000000000 --- a/test/WebSites/BasicWebSite/Controllers/ApplicationBaseController.cs +++ /dev/null @@ -1,10 +0,0 @@ -// 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. - -namespace BasicWebSite.Controllers -{ - public abstract class ApplicationBaseController - { - - } -} \ No newline at end of file diff --git a/test/WebSites/BasicWebSite/Controllers/Appointments.cs b/test/WebSites/BasicWebSite/Controllers/Appointments.cs deleted file mode 100644 index b34c70be79..0000000000 --- a/test/WebSites/BasicWebSite/Controllers/Appointments.cs +++ /dev/null @@ -1,20 +0,0 @@ -// 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 Microsoft.AspNetCore.Mvc; - -namespace BasicWebSite.Controllers -{ - [Route("/appointments")] - public class Appointments : ApplicationBaseController - { - [HttpGet("")] - public IActionResult Get() - { - return new ContentResult - { - Content = "2 appointments available." - }; - } - } -} \ No newline at end of file diff --git a/test/WebSites/ControllersFromServicesClassLibrary/ResourcesController.cs b/test/WebSites/ControllersFromServicesClassLibrary/ResourcesController.cs index a4108eaf8c..779e9f4ffc 100644 --- a/test/WebSites/ControllersFromServicesClassLibrary/ResourcesController.cs +++ b/test/WebSites/ControllersFromServicesClassLibrary/ResourcesController.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc; namespace ControllersFromServicesClassLibrary { + [Controller] [Route("/[controller]")] public class ResourcesController {