[Fixes #5352] When replacing Controller.Dispose with an explicit implementation the base Dispose is an action
This commit is contained in:
parent
7985121bab
commit
0cee00aae1
|
|
@ -390,7 +390,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
}
|
}
|
||||||
|
|
||||||
// Dispose method implemented from IDisposable is not valid
|
// Dispose method implemented from IDisposable is not valid
|
||||||
if (IsIDisposableMethod(methodInfo, typeInfo))
|
if (IsIDisposableMethod(methodInfo))
|
||||||
{
|
{
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
@ -627,11 +627,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
return selectorModel;
|
return selectorModel;
|
||||||
}
|
}
|
||||||
|
|
||||||
private bool IsIDisposableMethod(MethodInfo methodInfo, TypeInfo typeInfo)
|
private bool IsIDisposableMethod(MethodInfo methodInfo)
|
||||||
{
|
{
|
||||||
|
// Ideally we do not want Dispose method to be exposed as an action. However there are some scenarios where a user
|
||||||
|
// might want to expose a method with name "Dispose" (even though they might not be really disposing resources)
|
||||||
|
// Example: A controller deriving from MVC's Controller type might wish to have a method with name Dispose,
|
||||||
|
// in which case they can use the "new" keyword to hide the base controller's declaration.
|
||||||
|
|
||||||
|
// Find where the method was originally declared
|
||||||
|
var baseMethodInfo = methodInfo.GetBaseDefinition();
|
||||||
|
var declaringTypeInfo = baseMethodInfo.DeclaringType.GetTypeInfo();
|
||||||
|
|
||||||
return
|
return
|
||||||
(typeof(IDisposable).GetTypeInfo().IsAssignableFrom(typeInfo) &&
|
(typeof(IDisposable).GetTypeInfo().IsAssignableFrom(declaringTypeInfo) &&
|
||||||
typeInfo.GetRuntimeInterfaceMap(typeof(IDisposable)).TargetMethods[0] == methodInfo);
|
declaringTypeInfo.GetRuntimeInterfaceMap(typeof(IDisposable)).TargetMethods[0] == baseMethodInfo);
|
||||||
}
|
}
|
||||||
|
|
||||||
private bool IsSilentRouteAttribute(IRouteTemplateProvider routeTemplateProvider)
|
private bool IsSilentRouteAttribute(IRouteTemplateProvider routeTemplateProvider)
|
||||||
|
|
|
||||||
|
|
@ -883,6 +883,41 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
Assert.Contains(selectorModel.AttributeRouteModel.Attribute, action.Attributes);
|
Assert.Contains(selectorModel.AttributeRouteModel.Attribute, action.Attributes);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ControllerDispose_ExplicitlyImplemented_IDisposableMethods_AreTreatedAs_NonActions()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var builder = new TestApplicationModelProvider();
|
||||||
|
var typeInfo = typeof(DerivedFromControllerAndExplicitIDisposableImplementationController).GetTypeInfo();
|
||||||
|
var context = new ApplicationModelProviderContext(new[] { typeInfo });
|
||||||
|
|
||||||
|
// Act
|
||||||
|
builder.OnProvidersExecuting(context);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
var model = Assert.Single(context.Result.Controllers);
|
||||||
|
Assert.Empty(model.Actions);
|
||||||
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public void ControllerDispose_MethodsNamedDispose_AreTreatedAsActions()
|
||||||
|
{
|
||||||
|
// Arrange
|
||||||
|
var builder = new TestApplicationModelProvider();
|
||||||
|
var typeInfo = typeof(DerivedFromControllerAndHidesBaseDisposeMethodController).GetTypeInfo();
|
||||||
|
var context = new ApplicationModelProviderContext(new[] { typeInfo });
|
||||||
|
|
||||||
|
// Act
|
||||||
|
builder.OnProvidersExecuting(context);
|
||||||
|
|
||||||
|
// Assert
|
||||||
|
var model = Assert.Single(context.Result.Controllers);
|
||||||
|
var action = Assert.Single(model.Actions);
|
||||||
|
|
||||||
|
// Make sure that the Dispose method is from the derived controller and not the base 'Controller' type
|
||||||
|
Assert.Equal(typeInfo, action.ActionMethod.DeclaringType.GetTypeInfo());
|
||||||
|
}
|
||||||
|
|
||||||
private IList<AttributeRouteModel> GetAttributeRoutes(IList<SelectorModel> selectors)
|
private IList<AttributeRouteModel> GetAttributeRoutes(IList<SelectorModel> selectors)
|
||||||
{
|
{
|
||||||
return selectors
|
return selectors
|
||||||
|
|
@ -891,6 +926,23 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
.ToList();
|
.ToList();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private class DerivedFromControllerAndExplicitIDisposableImplementationController
|
||||||
|
: Mvc.Controller, IDisposable
|
||||||
|
{
|
||||||
|
void IDisposable.Dispose()
|
||||||
|
{
|
||||||
|
throw new NotImplementedException();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private class DerivedFromControllerAndHidesBaseDisposeMethodController : Mvc.Controller
|
||||||
|
{
|
||||||
|
public new void Dispose()
|
||||||
|
{
|
||||||
|
throw new NotImplementedException();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private class BaseClassWithAttributeRoutesController
|
private class BaseClassWithAttributeRoutesController
|
||||||
{
|
{
|
||||||
[Route("A")]
|
[Route("A")]
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue