[Fixes #3732] Simplify controller discovery.

* Introduce ControllerAttribute and use it to mark base classes as controllers.
* Changed rules for controller discovery to:
  * All controller types must be public, concrete, non open generic types.
  * NotController attribute is not applied to any type oif the hierarchy.
  * The type name ends with controller.
  * Controller attribute is applied to the type or to one of its ancestors.
This commit is contained in:
jacalvar 2016-03-18 21:00:11 -07:00
parent 680e9bb2d1
commit de9ffb13c7
11 changed files with 85 additions and 106 deletions

View File

@ -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
{
/// <summary>
/// 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
/// <see cref="NonControllerAttribute"/> is applied to any type in the hierarchy.
/// </summary>
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
public class ControllerAttribute : Attribute
{
}
}

View File

@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Mvc
/// <summary> /// <summary>
/// A base class for an MVC controller without view support. /// A base class for an MVC controller without view support.
/// </summary> /// </summary>
[Controller]
public abstract class ControllerBase public abstract class ControllerBase
{ {
private ControllerContext _controllerContext; private ControllerContext _controllerContext;

View File

@ -36,7 +36,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
{ {
var candidateAssemblies = new HashSet<Assembly>(_assemblyProvider.CandidateAssemblies); var candidateAssemblies = new HashSet<Assembly>(_assemblyProvider.CandidateAssemblies);
var types = candidateAssemblies.SelectMany(a => a.DefinedTypes); 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 <c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>. /// Returns <c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>.
/// </summary> /// </summary>
/// <param name="typeInfo">The <see cref="TypeInfo"/>.</param> /// <param name="typeInfo">The <see cref="TypeInfo"/>.</param>
/// <param name="candidateAssemblies">The set of candidate assemblies.</param>
/// <returns><c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>.</returns> /// <returns><c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>.</returns>
protected internal virtual bool IsController( protected internal virtual bool IsController(TypeInfo typeInfo)
TypeInfo typeInfo,
ISet<Assembly> candidateAssemblies)
{ {
if (typeInfo == null) if (typeInfo == null)
{ {
throw new ArgumentNullException(nameof(typeInfo)); throw new ArgumentNullException(nameof(typeInfo));
} }
if (candidateAssemblies == null)
{
throw new ArgumentNullException(nameof(candidateAssemblies));
}
if (!typeInfo.IsClass) if (!typeInfo.IsClass)
{ {
return false; return false;
} }
if (typeInfo.IsAbstract) if (typeInfo.IsAbstract)
{ {
return false; return false;
} }
// We only consider public top-level classes as controllers. IsPublic returns false for nested // We only consider public top-level classes as controllers. IsPublic returns false for nested
// classes, regardless of visibility modifiers // classes, regardless of visibility modifiers
if (!typeInfo.IsPublic) if (!typeInfo.IsPublic)
{ {
return false; return false;
} }
if (typeInfo.ContainsGenericParameters) if (typeInfo.ContainsGenericParameters)
{ {
return false; return false;
} }
if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) &&
!DerivesFromController(typeInfo, candidateAssemblies)) if (typeInfo.IsDefined(typeof(NonControllerAttribute)))
{ {
return false; return false;
} }
if (typeInfo.IsDefined(typeof(NonControllerAttribute)))
if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) &&
!typeInfo.IsDefined(typeof(ControllerAttribute)))
{ {
return false; return false;
} }
return true; return true;
} }
private bool DerivesFromController(TypeInfo typeInfo, ISet<Assembly> 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;
}
} }
} }

View File

@ -21,6 +21,7 @@ namespace System.Web.Http
[UseWebApiActionConventions] [UseWebApiActionConventions]
[UseWebApiParameterConventions] [UseWebApiParameterConventions]
[UseWebApiOverloading] [UseWebApiOverloading]
[Controller]
public abstract class ApiController : IDisposable public abstract class ApiController : IDisposable
{ {
private ControllerContext _controllerContext; private ControllerContext _controllerContext;

View File

@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -53,10 +53,10 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.True(isController);
} }
[Fact] [Fact]
@ -67,7 +67,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -81,7 +81,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -109,7 +109,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -123,7 +123,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -137,7 +137,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -151,7 +151,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -165,7 +165,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -179,7 +179,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -193,7 +193,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -207,7 +207,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -221,7 +221,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(typeInfo, CandidateAssemblies); var isController = provider.IsController(typeInfo);
// Assert // Assert
Assert.True(isController); Assert.True(isController);
@ -230,18 +230,31 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
[Theory] [Theory]
[InlineData(typeof(DescendantLevel1))] [InlineData(typeof(DescendantLevel1))]
[InlineData(typeof(DescendantLevel2))] [InlineData(typeof(DescendantLevel2))]
public void IsController_ReturnsTrue_IfAncestorTypeNameHasControllerSuffix(Type type) public void IsController_ReturnsTrue_IfAncestorTypeHasControllerAttribute(Type type)
{ {
// Arrange // Arrange
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies); var isController = provider.IsController(type.GetTypeInfo());
// Assert // Assert
Assert.True(isController); 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] [Theory]
[InlineData(typeof(BaseNonControllerController))] [InlineData(typeof(BaseNonControllerController))]
[InlineData(typeof(BaseNonControllerControllerChild))] [InlineData(typeof(BaseNonControllerControllerChild))]
@ -249,6 +262,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
[InlineData(typeof(BasePocoNonControllerControllerChild))] [InlineData(typeof(BasePocoNonControllerControllerChild))]
[InlineData(typeof(NonController))] [InlineData(typeof(NonController))]
[InlineData(typeof(NonControllerChild))] [InlineData(typeof(NonControllerChild))]
[InlineData(typeof(BaseNonControllerAttributeChildControllerControllerAttributeController))]
[InlineData(typeof(PersonModel))] // Verifies that POCO type hierarchies that don't derive from controller return false. [InlineData(typeof(PersonModel))] // Verifies that POCO type hierarchies that don't derive from controller return false.
public void IsController_ReturnsFalse_IfTypeOrAncestorHasNonControllerAttribute(Type type) public void IsController_ReturnsFalse_IfTypeOrAncestorHasNonControllerAttribute(Type type)
{ {
@ -256,7 +270,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
var provider = GetControllerTypeProvider(); var provider = GetControllerTypeProvider();
// Act // Act
var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies); var isController = provider.IsController(type.GetTypeInfo());
// Assert // Assert
Assert.False(isController); Assert.False(isController);
@ -297,11 +311,19 @@ namespace Microsoft.AspNetCore.Mvc.DefaultControllerTypeProviderControllers
{ {
} }
[Controller]
public abstract class Controller public abstract class Controller
{ {
} }
public abstract class NoControllerAttributeBaseController
{
}
public class NoSuffixNoControllerAttribute : NoControllerAttributeBaseController
{
}
public class OpenGenericController<T> : Controller public class OpenGenericController<T> : Controller
{ {
} }
@ -327,17 +349,19 @@ namespace Microsoft.AspNetCore.Mvc.DefaultControllerTypeProviderControllers
{ {
} }
public class CustomBaseController [Controller]
public class CustomBase
{ {
} }
[Controller]
public abstract class CustomAbstractBaseController 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 public class BaseNonControllerControllerChild : BaseNonControllerController
{ {

View File

@ -331,16 +331,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal("This is a basic website.", responseData); 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] [Fact]
public async Task TypesMarkedAsNonAction_AreInaccessible() public async Task TypesMarkedAsNonAction_AreInaccessible()
{ {

View File

@ -47,7 +47,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
} }
[Fact] [Fact]
public async Task TypesDerivingFromControllerPrefixedTypesAreRegistered() public async Task TypesDerivingFromTypesWithControllerAttributeAreRegistered()
{ {
// Arrange // Arrange
var expected = "4"; var expected = "4";

View File

@ -55,9 +55,9 @@ namespace System.Web.Http
// Assert // Assert
var controllerType = typeof(TestControllers.Blog).GetTypeInfo(); 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] [Fact]

View File

@ -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
{
}
}

View File

@ -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."
};
}
}
}

View File

@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc;
namespace ControllersFromServicesClassLibrary namespace ControllersFromServicesClassLibrary
{ {
[Controller]
[Route("/[controller]")] [Route("/[controller]")]
public class ResourcesController public class ResourcesController
{ {