Require attribute routing with [ApiController]

Fixes #6870

src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiControllerApplicationModelProvider.cs

@@ -33,9 +33,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         }
 
         /// <remarks>
-        /// Order is set to execute after the <see cref="DefaultApplicationModelProvider"/>.
+        /// Order is set to execute after the <see cref="DefaultApplicationModelProvider"/> and allow any other user
+        /// <see cref="IApplicationModelProvider"/> that configure routing to execute.
         /// </remarks>
-        public int Order => -1000 + 10;
+        public int Order => -1000 + 100;
 
         public void OnProvidersExecuted(ApplicationModelProviderContext context)
         {
@@ -45,21 +46,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         {
             foreach (var controllerModel in context.Result.Controllers)
             {
-                if (controllerModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
+                var isApiController = controllerModel.Attributes.OfType<IApiBehaviorMetadata>().Any();
-                {
+                var controllerHasSelectorModel = controllerModel.Selectors.Any(s => s.AttributeRouteModel != null);
-                    if (_apiBehaviorOptions.EnableModelStateInvalidFilter)
-                    {
-                        Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null);
-                        controllerModel.Filters.Add(_modelStateInvalidFilter);
-                    }
-
-                    continue;
-                }
 
                 foreach (var actionModel in controllerModel.Actions)
                 {
-                    if (actionModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
+                    if (isApiController || actionModel.Attributes.OfType<IApiBehaviorMetadata>().Any())
                     {
+                        if (!controllerHasSelectorModel && !actionModel.Selectors.Any(s => s.AttributeRouteModel != null))
+                        {
+                            // Require attribute routing with controllers annotated with ApiControllerAttribute
+                            throw new InvalidOperationException(Resources.FormatApiController_AttributeRouteRequired(nameof(ApiControllerAttribute)));
+                        }
+
                         if (_apiBehaviorOptions.EnableModelStateInvalidFilter)
                         {
                             Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null);

src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs

@@ -1340,6 +1340,20 @@ namespace Microsoft.AspNetCore.Mvc.Core
         internal static string FormatValidationProblemDescription_Title()
             => GetString("ValidationProblemDescription_Title");
 
+        /// <summary>
+        /// Action methods on controllers annotated with {0} must have an attribute route.
+        /// </summary>
+        internal static string ApiController_AttributeRouteRequired
+        {
+            get => GetString("ApiController_AttributeRouteRequired");
+        }
+
+        /// <summary>
+        /// Action methods on controllers annotated with {0} must have an attribute route.
+        /// </summary>
+        internal static string FormatApiController_AttributeRouteRequired(object p0)
+            => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0);
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

src/Microsoft.AspNetCore.Mvc.Core/Resources.resx

@@ -415,4 +415,7 @@
   <data name="ValidationProblemDescription_Title" xml:space="preserve">
     <value>One or more validation errors occured.</value>
   </data>
+  <data name="ApiController_AttributeRouteRequired" xml:space="preserve">
+    <value>Action methods on controllers annotated with {0} must have an attribute route.</value>
+  </data>
 </root>

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiControllerApplicationModelProviderTest.cs

@@ -22,15 +22,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             {
                 InvalidModelStateResponseFactory = _ => null,
             });
-            
+
             var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance);
 
             // Act
             provider.OnProvidersExecuting(context);
 
             // Assert
-            var controllerModel = Assert.Single(context.Result.Controllers);
+            var actionModel = Assert.Single(Assert.Single(context.Result.Controllers).Actions);
-            Assert.IsType<ModelStateInvalidFilter>(controllerModel.Filters.Last());
+            Assert.IsType<ModelStateInvalidFilter>(actionModel.Filters.Last());
         }
 
         [Fact]
@@ -109,6 +109,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 });
         }
 
+        [Fact]
+        public void OnProvidersExecuting_ThrowsIfControllerWithAttribute_HasActionsWithoutAttributeRouting()
+        {
+            // Arrange
+            var context = GetContext(typeof(ActionsWithoutAttributeRouting));
+            var options = new TestOptionsManager<ApiBehaviorOptions>(new ApiBehaviorOptions
+            {
+                InvalidModelStateResponseFactory = _ => null,
+            });
+
+            var provider = new ApiControllerApplicationModelProvider(options, NullLoggerFactory.Instance);
+
+            // Act & Assert
+            var ex = Assert.Throws<InvalidOperationException>(() => provider.OnProvidersExecuting(context));
+            Assert.Equal(
+                "Action methods on controllers annotated with ApiControllerAttribute must have an attribute route.",
+                ex.Message);
+        }
+
         private static ApplicationModelProviderContext GetContext(Type type)
         {
             var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() });
@@ -117,20 +136,28 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         }
 
         [ApiController]
+        [Route("TestApi")]
         private class TestApiController : Controller
         {
+            [HttpGet]
             public IActionResult TestAction() => null;
         }
 
-        
         private class SimpleController : Controller
         {
             public IActionResult ActionWithoutFilter() => null;
 
             [TestApiBehavior]
+            [HttpGet("/Simple/ActionWithFilter")]
             public IActionResult ActionWithFilter() => null;
         }
 
+        [ApiController]
+        private class ActionsWithoutAttributeRouting
+        {
+            public IActionResult Index() => null;
+        }
+
         [AttributeUsage(AttributeTargets.Method)]
         private class TestApiBehavior : Attribute, IApiBehaviorMetadata
         {