Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit eaaa2c0

Browse files
javiercnjaviercn
committed
[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.
1 parent fb81a5e commit eaaa2c0

File tree

11 files changed

+85
-106
lines changed

11 files changed

+85
-106
lines changed

‎src/Microsoft.AspNetCore.Mvc.Core/ControllerAttribute.cs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
6+
namespace Microsoft.AspNetCore.Mvc
7+
{
8+
/// <summary>
9+
/// Indicates that the type and any derived types that this attribute is applied to
10+
/// are considered a controller by the default controller discovery mechanism, unless
11+
/// <see cref="NonControllerAttribute"/> is applied to any type in the hierarchy.
12+
/// </summary>
13+
[AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)]
14+
public class ControllerAttribute : Attribute
15+
{
16+
}
17+
}

‎src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Mvc
2323
/// <summary>
2424
/// A base class for an MVC controller without view support.
2525
/// </summary>
26+
[Controller]
2627
public abstract class ControllerBase
2728
{
2829
private ControllerContext _controllerContext;

‎src/Microsoft.AspNetCore.Mvc.Core/Controllers/DefaultControllerTypeProvider.cs‎

Lines changed: 10 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,87 +36,56 @@ public virtual IEnumerable<TypeInfo> ControllerTypes
3636
{
3737
var candidateAssemblies = new HashSet<Assembly>(_assemblyProvider.CandidateAssemblies);
3838
var types = candidateAssemblies.SelectMany(a => a.DefinedTypes);
39-
return types.Where(typeInfo => IsController(typeInfo, candidateAssemblies));
39+
return types.Where(typeInfo => IsController(typeInfo));
4040
}
4141
}
4242

4343
/// <summary>
4444
/// Returns <c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>.
4545
/// </summary>
4646
/// <param name="typeInfo">The <see cref="TypeInfo"/>.</param>
47-
/// <param name="candidateAssemblies">The set of candidate assemblies.</param>
4847
/// <returns><c>true</c> if the <paramref name="typeInfo"/> is a controller. Otherwise <c>false</c>.</returns>
49-
protected internal virtual bool IsController(
50-
TypeInfo typeInfo,
51-
ISet<Assembly> candidateAssemblies)
48+
protected internal virtual bool IsController(TypeInfo typeInfo)
5249
{
5350
if (typeInfo == null)
5451
{
5552
throw new ArgumentNullException(nameof(typeInfo));
5653
}
5754

58-
if (candidateAssemblies == null)
59-
{
60-
throw new ArgumentNullException(nameof(candidateAssemblies));
61-
}
62-
6355
if (!typeInfo.IsClass)
6456
{
6557
return false;
6658
}
59+
6760
if (typeInfo.IsAbstract)
6861
{
6962
return false;
7063
}
64+
7165
// We only consider public top-level classes as controllers. IsPublic returns false for nested
7266
// classes, regardless of visibility modifiers
7367
if (!typeInfo.IsPublic)
7468
{
7569
return false;
7670
}
71+
7772
if (typeInfo.ContainsGenericParameters)
7873
{
7974
return false;
8075
}
81-
if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) &&
82-
!DerivesFromController(typeInfo, candidateAssemblies))
83-
{
84-
return false;
85-
}
76+
8677
if (typeInfo.IsDefined(typeof(NonControllerAttribute)))
8778
{
8879
return false;
8980
}
9081

91-
return true;
92-
}
93-
94-
private bool DerivesFromController(TypeInfo typeInfo, ISet<Assembly> candidateAssemblies)
95-
{
96-
while (typeInfo != ObjectTypeInfo)
82+
if (!typeInfo.Name.EndsWith(ControllerTypeName, StringComparison.OrdinalIgnoreCase) &&
83+
!typeInfo.IsDefined(typeof(ControllerAttribute)))
9784
{
98-
var baseTypeInfo = typeInfo.BaseType.GetTypeInfo();
99-
100-
// A base type will be treated as a controller if
101-
// a) it ends in the term "Controller" and
102-
// b) it's assembly is one of the candidate assemblies we're considering. This ensures that the assembly
103-
// the base type is declared in references Mvc.
104-
if (baseTypeInfo.Name.EndsWith(ControllerTypeName, StringComparison.Ordinal) &&
105-
candidateAssemblies.Contains(baseTypeInfo.Assembly))
106-
{
107-
return true;
108-
}
109-
110-
// c). The base type is called 'Controller.
111-
if (string.Equals(baseTypeInfo.Name, ControllerTypeName, StringComparison.Ordinal))
112-
{
113-
return true;
114-
}
115-
116-
typeInfo = baseTypeInfo;
85+
return false;
11786
}
11887

119-
return false;
88+
return true;
12089
}
12190
}
12291
}

‎src/Microsoft.AspNetCore.Mvc.WebApiCompatShim/ApiController.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ namespace System.Web.Http
2121
[UseWebApiActionConventions]
2222
[UseWebApiParameterConventions]
2323
[UseWebApiOverloading]
24+
[Controller]
2425
public abstract class ApiController : IDisposable
2526
{
2627
private ControllerContext _controllerContext;

‎test/Microsoft.AspNetCore.Mvc.Core.Test/Controllers/DefaultControllerTypeProviderTest.cs‎

Lines changed: 52 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public void IsController_UserDefinedClass_DerivedFromController()
2525
var provider = GetControllerTypeProvider();
2626

2727
// Act
28-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
28+
var isController = provider.IsController(typeInfo);
2929

3030
// Assert
3131
Assert.True(isController);
@@ -39,7 +39,7 @@ public void IsController_UserDefinedClass_DerivedFromControllerBase()
3939
var provider = GetControllerTypeProvider();
4040

4141
// Act
42-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
42+
var isController = provider.IsController(typeInfo);
4343

4444
// Assert
4545
Assert.True(isController);
@@ -53,10 +53,10 @@ public void IsController_UserDefinedClass_DerivedFromControllerBase_WithoutSuffi
5353
var provider = GetControllerTypeProvider();
5454

5555
// Act
56-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
56+
var isController = provider.IsController(typeInfo);
5757

5858
// Assert
59-
Assert.False(isController);
59+
Assert.True(isController);
6060
}
6161

6262
[Fact]
@@ -67,7 +67,7 @@ public void IsController_FrameworkControllerClass()
6767
var provider = GetControllerTypeProvider();
6868

6969
// Act
70-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
70+
var isController = provider.IsController(typeInfo);
7171

7272
// Assert
7373
Assert.False(isController);
@@ -81,7 +81,7 @@ public void IsController_FrameworkBaseControllerClass()
8181
var provider = GetControllerTypeProvider();
8282

8383
// Act
84-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
84+
var isController = provider.IsController(typeInfo);
8585

8686
// Assert
8787
Assert.False(isController);
@@ -95,7 +95,7 @@ public void IsController_UserDefinedControllerClass()
9595
var provider = GetControllerTypeProvider();
9696

9797
// Act
98-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
98+
var isController = provider.IsController(typeInfo);
9999

100100
// Assert
101101
Assert.False(isController);
@@ -109,7 +109,7 @@ public void IsController_Interface()
109109
var provider = GetControllerTypeProvider();
110110

111111
// Act
112-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
112+
var isController = provider.IsController(typeInfo);
113113

114114
// Assert
115115
Assert.False(isController);
@@ -123,7 +123,7 @@ public void IsController_AbstractClass()
123123
var provider = GetControllerTypeProvider();
124124

125125
// Act
126-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
126+
var isController = provider.IsController(typeInfo);
127127

128128
// Assert
129129
Assert.False(isController);
@@ -137,7 +137,7 @@ public void IsController_DerivedAbstractClass()
137137
var provider = GetControllerTypeProvider();
138138

139139
// Act
140-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
140+
var isController = provider.IsController(typeInfo);
141141

142142
// Assert
143143
Assert.True(isController);
@@ -151,7 +151,7 @@ public void IsController_OpenGenericClass()
151151
var provider = GetControllerTypeProvider();
152152

153153
// Act
154-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
154+
var isController = provider.IsController(typeInfo);
155155

156156
// Assert
157157
Assert.False(isController);
@@ -165,7 +165,7 @@ public void IsController_WithoutSuffixOrAncestorWithController()
165165
var provider = GetControllerTypeProvider();
166166

167167
// Act
168-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
168+
var isController = provider.IsController(typeInfo);
169169

170170
// Assert
171171
Assert.False(isController);
@@ -179,7 +179,7 @@ public void IsController_ClosedGenericClass()
179179
var provider = GetControllerTypeProvider();
180180

181181
// Act
182-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
182+
var isController = provider.IsController(typeInfo);
183183

184184
// Assert
185185
Assert.True(isController);
@@ -193,7 +193,7 @@ public void IsController_DerivedGenericClass()
193193
var provider = GetControllerTypeProvider();
194194

195195
// Act
196-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
196+
var isController = provider.IsController(typeInfo);
197197

198198
// Assert
199199
Assert.True(isController);
@@ -207,7 +207,7 @@ public void IsController_Poco_WithNamingConvention()
207207
var provider = GetControllerTypeProvider();
208208

209209
// Act
210-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
210+
var isController = provider.IsController(typeInfo);
211211

212212
// Assert
213213
Assert.True(isController);
@@ -221,7 +221,7 @@ public void IsController_NoControllerSuffix()
221221
var provider = GetControllerTypeProvider();
222222

223223
// Act
224-
var isController = provider.IsController(typeInfo, CandidateAssemblies);
224+
var isController = provider.IsController(typeInfo);
225225

226226
// Assert
227227
Assert.True(isController);
@@ -230,33 +230,47 @@ public void IsController_NoControllerSuffix()
230230
[Theory]
231231
[InlineData(typeof(DescendantLevel1))]
232232
[InlineData(typeof(DescendantLevel2))]
233-
public void IsController_ReturnsTrue_IfAncestorTypeNameHasControllerSuffix(Type type)
233+
public void IsController_ReturnsTrue_IfAncestorTypeHasControllerAttribute(Type type)
234234
{
235235
// Arrange
236236
var provider = GetControllerTypeProvider();
237237

238238
// Act
239-
var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies);
239+
var isController = provider.IsController(type.GetTypeInfo());
240240

241241
// Assert
242242
Assert.True(isController);
243243
}
244244

245+
[Fact]
246+
public void IsController_ReturnsFalse_IfAncestorTypeDoesNotHaveControllerAttribute()
247+
{
248+
// Arrange
249+
var provider = GetControllerTypeProvider();
250+
251+
// Act
252+
var isController = provider.IsController(typeof(NoSuffixNoControllerAttribute).GetTypeInfo());
253+
254+
// Assert
255+
Assert.False(isController);
256+
}
257+
245258
[Theory]
246259
[InlineData(typeof(BaseNonControllerController))]
247260
[InlineData(typeof(BaseNonControllerControllerChild))]
248261
[InlineData(typeof(BasePocoNonControllerController))]
249262
[InlineData(typeof(BasePocoNonControllerControllerChild))]
250263
[InlineData(typeof(NonController))]
251264
[InlineData(typeof(NonControllerChild))]
265+
[InlineData(typeof(BaseNonControllerAttributeChildControllerControllerAttributeController))]
252266
[InlineData(typeof(PersonModel))] // Verifies that POCO type hierarchies that don't derive from controller return false.
253267
public void IsController_ReturnsFalse_IfTypeOrAncestorHasNonControllerAttribute(Type type)
254268
{
255269
// Arrange
256270
var provider = GetControllerTypeProvider();
257271

258272
// Act
259-
var isController = provider.IsController(type.GetTypeInfo(), CandidateAssemblies);
273+
var isController = provider.IsController(type.GetTypeInfo());
260274

261275
// Assert
262276
Assert.False(isController);
@@ -297,11 +311,19 @@ public class Products : ControllerBase
297311
{
298312
}
299313

300-
314+
[Controller]
301315
public abstract class Controller
302316
{
303317
}
304318

319+
public abstract class NoControllerAttributeBaseController
320+
{
321+
}
322+
323+
public class NoSuffixNoControllerAttribute : NoControllerAttributeBaseController
324+
{
325+
}
326+
305327
public class OpenGenericController<T> : Controller
306328
{
307329
}
@@ -327,17 +349,19 @@ public class PocoController
327349
{
328350
}
329351

330-
public class CustomBaseController
352+
[Controller]
353+
public class CustomBase
331354
{
332355

333356
}
334357

358+
[Controller]
335359
public abstract class CustomAbstractBaseController
336360
{
337361

338362
}
339363

340-
public class DescendantLevel1 : CustomBaseController
364+
public class DescendantLevel1 : CustomBase
341365
{
342366

343367
}
@@ -369,6 +393,12 @@ public class BaseNonControllerController : Controller
369393

370394
}
371395

396+
[Controller]
397+
public class BaseNonControllerAttributeChildControllerControllerAttributeController : BaseNonControllerController
398+
{
399+
400+
}
401+
372402
public class BaseNonControllerControllerChild : BaseNonControllerController
373403
{
374404

‎test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -331,16 +331,6 @@ public async Task ConfigureMvc_AddsOptionsProperly()
331331
Assert.Equal("This is a basic website.", responseData);
332332
}
333333

334-
[Fact]
335-
public async Task TypesWithoutControllerSuffix_DerivingFromTypesWithControllerSuffix_CanBeAccessed()
336-
{
337-
// Act
338-
var response = await Client.GetStringAsync("appointments");
339-
340-
// Assert
341-
Assert.Equal("2 appointments available.", response);
342-
}
343-
344334
[Fact]
345335
public async Task TypesMarkedAsNonAction_AreInaccessible()
346336
{

0 commit comments

Comments
 (0)