diff --git a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultActionModelBuilder.cs b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultActionModelBuilder.cs index 0af769df30..b9c653eaea 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultActionModelBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultActionModelBuilder.cs @@ -37,10 +37,60 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels return Enumerable.Empty(); } + // For attribute routes on a action, we want want to support 'overriding' routes on a + // virtual method, but allow 'overriding'. So we need to walk up the hierarchy looking + // for the first definition to define routes. + // + // Then we want to 'filter' the set of attributes, so that only the effective routes apply. + var currentMethodInfo = methodInfo; + + IRouteTemplateProvider[] routeAttributes = null; + + while (true) + { + routeAttributes = currentMethodInfo + .GetCustomAttributes(inherit: false) + .OfType() + .ToArray(); + + if (routeAttributes.Length > 0) + { + // Found 1 or more route attributes. + break; + } + + // GetBaseDefinition returns 'this' when it gets to the bottom of the chain. + var nextMethodInfo = currentMethodInfo.GetBaseDefinition(); + if (currentMethodInfo == nextMethodInfo) + { + break; + } + + currentMethodInfo = nextMethodInfo; + } + // CoreCLR returns IEnumerable from GetCustomAttributes - the OfType // is needed to so that the result of ToArray() is object var attributes = methodInfo.GetCustomAttributes(inherit: true).OfType().ToArray(); + // This is fairly complicated so that we maintain referential equality between items in + // ActionModel.Attributes and ActionModel.Attributes[*].Attribute. + var applicableAttributes = new List(); + foreach (var attribute in attributes) + { + if (attribute is IRouteTemplateProvider) + { + // This attribute is a route-attribute, leave it out. + } + else + { + applicableAttributes.Add(attribute); + } + } + applicableAttributes.AddRange(routeAttributes); + + attributes = applicableAttributes.ToArray(); + // Route attributes create multiple actions, we want to split the set of // attributes based on these so each action only has the attributes that affect it. // diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultActionModelBuilderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultActionModelBuilderTest.cs index 23daba70d3..cb39b06884 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultActionModelBuilderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultActionModelBuilderTest.cs @@ -706,6 +706,52 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels Assert.Equal(new string[] { "GET" }, action.HttpMethods); } + [Fact] + public void GetActions_InheritedAttributeRoutes() + { + // Arrange + var builder = CreateTestDefaultActionModelBuilder(); + var typeInfo = typeof(DerivedClassInheritsAttributeRoutesController).GetTypeInfo(); + var actionName = nameof(DerivedClassInheritsAttributeRoutesController.Edit); + + // Act + var actions = builder.BuildActionModels(typeInfo, typeInfo.GetMethod(actionName)); + + // Assert + Assert.Equal(2, actions.Count()); + + var action = Assert.Single(actions, a => a.AttributeRouteModel?.Template == "A"); + Assert.Equal(1, action.Attributes.Count); + Assert.Contains(action.AttributeRouteModel.Attribute, action.Attributes); + + action = Assert.Single(actions, a => a.AttributeRouteModel?.Template == "B"); + Assert.Equal(1, action.Attributes.Count); + Assert.Contains(action.AttributeRouteModel.Attribute, action.Attributes); + } + + [Fact] + public void GetActions_InheritedAttributeRoutesOverridden() + { + // Arrange + var builder = CreateTestDefaultActionModelBuilder(); + var typeInfo = typeof(DerivedClassOverridesAttributeRoutesController).GetTypeInfo(); + var actionName = nameof(DerivedClassOverridesAttributeRoutesController.Edit); + + // Act + var actions = builder.BuildActionModels(typeInfo, typeInfo.GetMethod(actionName)); + + // Assert + Assert.Equal(2, actions.Count()); + + var action = Assert.Single(actions, a => a.AttributeRouteModel?.Template == "C"); + Assert.Equal(1, action.Attributes.Count); + Assert.Contains(action.AttributeRouteModel.Attribute, action.Attributes); + + action = Assert.Single(actions, a => a.AttributeRouteModel?.Template == "D"); + Assert.Equal(1, action.Attributes.Count); + Assert.Contains(action.AttributeRouteModel.Attribute, action.Attributes); + } + private static DefaultActionModelBuilder CreateTestDefaultActionModelBuilder( AuthorizationOptions authOptions = null) { @@ -731,6 +777,31 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels } } + private class BaseClassWithAttributeRoutesController + { + [Route("A")] + [Route("B")] + public virtual void Edit() + { + } + } + + private class DerivedClassInheritsAttributeRoutesController : BaseClassWithAttributeRoutesController + { + public override void Edit() + { + } + } + + private class DerivedClassOverridesAttributeRoutesController : BaseClassWithAttributeRoutesController + { + [Route("C")] + [Route("D")] + public override void Edit() + { + } + } + private class BaseController : Controller { public void GetFromBase() // Valid action method.