Part 1 of fix for #2248 - Ambiguity when inheriting attribute routes

The change here is to make an attribute route defined on a base class
inherited only if no other routes were defined 'closer' to the controller
class.

To put another way, attribute routes can either be inherited or
overridden, you can't inherit + add your own.

src/Microsoft.AspNet.Mvc.Core/ApplicationModels/DefaultControllerModelBuilder.cs

@@ -76,10 +76,57 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels
         /// <returns>A <see cref="ControllerModel"/> for the given <see cref="TypeInfo"/>.</returns>
         protected virtual ControllerModel CreateControllerModel([NotNull] TypeInfo typeInfo)
         {
+            // For attribute routes on a controller, we want want to support 'overriding' routes on a derived
+            // class. So we need to walk up the hierarchy looking for the first class to define routes.
+            //
+            // Then we want to 'filter' the set of attributes, so that only the effective routes apply.
+            var currentTypeInfo = typeInfo;
+            var objectTypeInfo = typeof(object).GetTypeInfo();
+
+            IRouteTemplateProvider[] routeAttributes = null;
+
+            do
+            {
+                routeAttributes = currentTypeInfo
+                        .GetCustomAttributes(inherit: false)
+                        .OfType<IRouteTemplateProvider>()
+                        .ToArray();
+
+                if (routeAttributes.Length > 0)
+                {
+                    // Found 1 or more route attributes.
+                    break;
+                }
+
+                currentTypeInfo = currentTypeInfo.BaseType.GetTypeInfo();
+            }
+            while (currentTypeInfo != objectTypeInfo);
+
             // CoreCLR returns IEnumerable<Attribute> from GetCustomAttributes - the OfType<object>
             // is needed to so that the result of ToArray() is object
             var attributes = typeInfo.GetCustomAttributes(inherit: true).OfType<object>().ToArray();
+
+            // This is fairly complicated so that we maintain referential equality between items in
+            // ControllerModel.Attributes and ControllerModel.Attributes[*].Attribute.
+            var filteredAttributes = new List<object>();
+            foreach (var attribute in attributes)
+            {
+                if (attribute is IRouteTemplateProvider)
+                {
+                    // This attribute is a route-attribute, leave it out.
+                }
+                else
+                {
+                    filteredAttributes.Add(attribute);
+                }
+            }
+            filteredAttributes.AddRange(routeAttributes);
+
+            attributes = filteredAttributes.ToArray();
+
             var controllerModel = new ControllerModel(typeInfo, attributes);
+            AddRange(
+                controllerModel.AttributeRoutes, routeAttributes.Select(a => new AttributeRouteModel(a)));
 
             controllerModel.ControllerName =
                 typeInfo.Name.EndsWith("Controller", StringComparison.OrdinalIgnoreCase) ?
@@ -108,10 +155,6 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels
                 controllerModel.Filters.Add(new AuthorizeFilter(policy));
             }
 
-            AddRange(
-                controllerModel.AttributeRoutes,
-                attributes.OfType<IRouteTemplateProvider>().Select(rtp => new AttributeRouteModel(rtp)));
-
             var apiVisibility = attributes.OfType<IApiDescriptionVisibilityProvider>().FirstOrDefault();
             if (apiVisibility != null)
             {

src/Microsoft.AspNet.Mvc.Core/ControllerActionDescriptorBuilder.cs

@@ -11,7 +11,6 @@ using Microsoft.AspNet.Mvc.ApiExplorer;
 using Microsoft.AspNet.Mvc.ApplicationModels;
 using Microsoft.AspNet.Mvc.Core;
 using Microsoft.AspNet.Mvc.Routing;
-using Microsoft.AspNet.Mvc.ModelBinding;
 
 namespace Microsoft.AspNet.Mvc
 {

test/Microsoft.AspNet.Mvc.Core.Test/ApplicationModel/DefaultControllerModelBuilderTest.cs

@@ -152,6 +152,64 @@ namespace Microsoft.AspNet.Mvc.ApplicationModels
             Assert.Empty(model.Filters);
         }
 
+        [Fact]
+        public void BuildControllerModel_ClassWithInheritedRoutes()
+        {
+            // Arrange
+            var builder = new DefaultControllerModelBuilder(new DefaultActionModelBuilder(null), null);
+            var typeInfo = typeof(DerivedClassInheritingRoutesController).GetTypeInfo();
+
+            // Act
+            var model = builder.BuildControllerModel(typeInfo);
+
+            // Assert
+            Assert.Equal(2, model.AttributeRoutes.Count);
+            Assert.Equal(2, model.Attributes.Count);
+
+            var route = Assert.Single(model.AttributeRoutes, r => r.Template == "A");
+            Assert.Contains(route.Attribute, model.Attributes);
+
+            route = Assert.Single(model.AttributeRoutes, r => r.Template == "B");
+            Assert.Contains(route.Attribute, model.Attributes);
+        }
+
+        [Fact]
+        public void BuildControllerModel_ClassWithHiddenInheritedRoutes()
+        {
+            // Arrange
+            var builder = new DefaultControllerModelBuilder(new DefaultActionModelBuilder(null), null);
+            var typeInfo = typeof(DerivedClassHidingRoutesController).GetTypeInfo();
+
+            // Act
+            var model = builder.BuildControllerModel(typeInfo);
+
+            // Assert
+            Assert.Equal(2, model.AttributeRoutes.Count);
+            Assert.Equal(2, model.Attributes.Count);
+
+            var route = Assert.Single(model.AttributeRoutes, r => r.Template == "C");
+            Assert.Contains(route.Attribute, model.Attributes);
+
+            route = Assert.Single(model.AttributeRoutes, r => r.Template == "D");
+            Assert.Contains(route.Attribute, model.Attributes);
+        }
+
+        [Route("A")]
+        [Route("B")]
+        private class BaseClassWithRoutesController
+        {
+        }
+
+        private class DerivedClassInheritingRoutesController : BaseClassWithRoutesController
+        {
+        }
+
+        [Route("C")]
+        [Route("D")]
+        private class DerivedClassHidingRoutesController : BaseClassWithRoutesController
+        {
+        }
+
         private class StoreController : Mvc.Controller
         {
         }

test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionDescriptorProviderTests.cs

@@ -1391,6 +1391,7 @@ namespace Microsoft.AspNet.Mvc.Test
         {
             // Arrange
             var actionName = nameof(MultipleRouteProviderOnActionAndControllerController.Delete);
+
             var provider = GetProvider(typeof(MultipleRouteProviderOnActionAndControllerController).GetTypeInfo());
 
             // Act

test/WebSites/RoutingWebSite/RoutingWebSite.xproj

@@ -10,6 +10,12 @@
     <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">..\..\..\artifacts\obj\$(MSBuildProjectName)</BaseIntermediateOutputPath>
     <OutputPath Condition="'$(OutputPath)'=='' ">..\..\..\artifacts\bin\$(MSBuildProjectName)\</OutputPath>
   </PropertyGroup>
+  <PropertyGroup Label="Configuration" Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
+    <AssemblyName>RoutingWebSite</AssemblyName>
+  </PropertyGroup>
+  <PropertyGroup Label="Configuration">
+    <RootNamespace>RoutingWebSite</RootNamespace>
+  </PropertyGroup>
   <PropertyGroup>
     <SchemaVersion>2.0</SchemaVersion>
     <DevelopmentServerPort>49632</DevelopmentServerPort>