Error message for [ApiController] without attribute route needs to be better

Fixes #7277

build/dependencies.props

@@ -69,6 +69,7 @@
     <MicrosoftExtensionsPropertyActivatorSourcesPackageVersion>2.1.0-preview1-28157</MicrosoftExtensionsPropertyActivatorSourcesPackageVersion>
     <MicrosoftExtensionsPropertyHelperSourcesPackageVersion>2.1.0-preview1-28157</MicrosoftExtensionsPropertyHelperSourcesPackageVersion>
     <MicrosoftExtensionsSecurityHelperSourcesPackageVersion>2.1.0-preview1-28157</MicrosoftExtensionsSecurityHelperSourcesPackageVersion>
+    <MicrosoftExtensionsTypeNameHelperSourcesPackageVersion>2.1.0-preview1-28157</MicrosoftExtensionsTypeNameHelperSourcesPackageVersion>
     <MicrosoftExtensionsWebEncodersPackageVersion>2.1.0-preview1-28157</MicrosoftExtensionsWebEncodersPackageVersion>
     <MicrosoftNETCoreApp20PackageVersion>2.0.0</MicrosoftNETCoreApp20PackageVersion>
     <MicrosoftNETCoreApp21PackageVersion>2.1.0-preview1-26122-01</MicrosoftNETCoreApp21PackageVersion>

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs

@@ -9,10 +9,11 @@ using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
-    [DebuggerDisplay("{Controller.ControllerType.Name}.{ActionMethod.Name}")]
+    [DebuggerDisplay("{DisplayName}")]
     public class ActionModel : ICommonModel, IFilterModel, IApiExplorerModel
     {
         public ActionModel(
@@ -123,5 +124,20 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
         string ICommonModel.Name => ActionName;
 
         public IList<SelectorModel> Selectors { get; }
+
+        public string DisplayName
+        {
+            get
+            {
+                if (Controller == null)
+                {
+                    return ActionMethod.Name;
+                }
+
+                var controllerType = TypeNameHelper.GetTypeDisplayName(Controller.ControllerType);
+                var controllerAssembly = Controller?.ControllerType.Assembly.GetName().Name;
+                return $"{controllerType}.{ActionMethod.Name} ({controllerAssembly})";
+            }
+        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerModel.cs

@@ -8,10 +8,11 @@ using System.Linq;
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.ApplicationModels
 {
-    [DebuggerDisplay("Name={ControllerName}, Type={ControllerType.Name}")]
+    [DebuggerDisplay("{DisplayName}")]
     public class ControllerModel : ICommonModel, IFilterModel, IApiExplorerModel
     {
         public ControllerModel(
@@ -118,5 +119,15 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
         public IDictionary<object, object> Properties { get; }
 
         public IList<SelectorModel> Selectors { get; }
+
+        public string DisplayName
+        {
+            get
+            {
+                var controllerType = TypeNameHelper.GetTypeDisplayName(ControllerType);
+                var controllerAssembly = ControllerType.Assembly.GetName().Name;
+                return $"{controllerType} ({controllerAssembly})";
+            }
+        }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/Controllers/ControllerActionDescriptor.cs

@@ -6,6 +6,7 @@ using System.Diagnostics;
 using System.Globalization;
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.Extensions.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.Controllers
 {
@@ -29,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
                     base.DisplayName = string.Format(
                         CultureInfo.InvariantCulture,
                         "{0}.{1} ({2})",
-                        ControllerTypeInfo.FullName,
+                        TypeNameHelper.GetTypeDisplayName(ControllerTypeInfo),
                         MethodInfo.Name,
                         ControllerTypeInfo.Assembly.GetName().Name);
                 }

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

@@ -114,7 +114,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             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)));
+                var message = Resources.FormatApiController_AttributeRouteRequired(
+                     actionModel.DisplayName,
+                    nameof(ApiControllerAttribute));
+                throw new InvalidOperationException(message);
             }
         }
 

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

@@ -298,7 +298,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 actionModel.RouteValues.Add(routeValueProvider.RouteKey, routeValueProvider.RouteValue);
             }
 
-            //TODO: modify comment
             // Now we need to determine the action selection info (cross-section of routes and constraints)
             //
             // For attribute routes on a action, we want to support 'overriding' routes on a
@@ -313,9 +312,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             while (true)
             {
                 routeAttributes = currentMethodInfo
-                        .GetCustomAttributes(inherit: false)
+                    .GetCustomAttributes(inherit: false)
-                        .OfType<IRouteTemplateProvider>()
+                    .OfType<IRouteTemplateProvider>()
-                        .ToArray();
+                    .ToArray();
 
                 if (routeAttributes.Length > 0)
                 {
@@ -679,4 +678,4 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 typeof(IEnumerable<IFormFile>).IsAssignableFrom(parameterType);
         }
     }
-}
+}

src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj

@@ -37,6 +37,7 @@ Microsoft.AspNetCore.Mvc.RouteAttribute</Description>
     <PackageReference Include="Microsoft.Extensions.PropertyActivator.Sources" PrivateAssets="All" Version="$(MicrosoftExtensionsPropertyActivatorSourcesPackageVersion)" />
     <PackageReference Include="Microsoft.Extensions.PropertyHelper.Sources" PrivateAssets="All" Version="$(MicrosoftExtensionsPropertyHelperSourcesPackageVersion)" />
     <PackageReference Include="Microsoft.Extensions.SecurityHelper.Sources" PrivateAssets="All" Version="$(MicrosoftExtensionsSecurityHelperSourcesPackageVersion)" />
+    <PackageReference Include="Microsoft.Extensions.TypeNameHelper.Sources" PrivateAssets="All" Version="$(MicrosoftExtensionsTypeNameHelperSourcesPackageVersion)" />
     <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="$(SystemDiagnosticsDiagnosticSourcePackageVersion)" />
     <PackageReference Include="System.Threading.Tasks.Extensions" Version="$(SystemThreadingTasksExtensionsPackageVersion)" />
   </ItemGroup>

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

@@ -1341,7 +1341,7 @@ namespace Microsoft.AspNetCore.Mvc.Core
             => GetString("ValidationProblemDescription_Title");
 
         /// <summary>
-        /// Action methods on controllers annotated with {0} must have an attribute route.
+        /// Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed.
         /// </summary>
         internal static string ApiController_AttributeRouteRequired
         {
@@ -1349,10 +1349,10 @@ namespace Microsoft.AspNetCore.Mvc.Core
         }
 
         /// <summary>
-        /// Action methods on controllers annotated with {0} must have an attribute route.
+        /// Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed.
         /// </summary>
-        internal static string FormatApiController_AttributeRouteRequired(object p0)
+        internal static string FormatApiController_AttributeRouteRequired(object p0, object p1)
-            => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0);
+            => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0, p1);
 
         /// <summary>
         /// No file provider has been configured to process the supplied file.

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

@@ -416,9 +416,9 @@
     <value>One or more validation errors occurred.</value>
   </data>
   <data name="ApiController_AttributeRouteRequired" xml:space="preserve">
-    <value>Action methods on controllers annotated with {0} must have an attribute route.</value>
+    <value>Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed.</value>
   </data>
   <data name="VirtualFileResultExecutor_NoFileProviderConfigured" xml:space="preserve">
     <value>No file provider has been configured to process the supplied file.</value>
   </data>
-</root>
+</root>

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs

@@ -125,6 +125,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
                     // Ensure non-default value
                     Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType));
                 }
+                else if (property.Name.Equals(nameof(ActionModel.DisplayName)))
+                {
+                    // DisplayName is re-calculated, hence reference equality wouldn't work.
+                    Assert.Equal(value1, value2);
+                }
                 else
                 {
                     Assert.Same(value1, value2);
@@ -153,4 +158,4 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
             public string RouteValue { get; set; }
         }
     }
-}
+}

test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs

@@ -132,6 +132,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
                     // Ensure non-default value
                     Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType));
                 }
+                else if (property.Name.Equals(nameof(ControllerModel.DisplayName)))
+                {
+                    // DisplayName is re-calculated, hence reference equality wouldn't work.
+                    Assert.Equal(value1, value2);
+                }
                 else
                 {
                     Assert.Same(value1, value2);

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

@@ -148,14 +148,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         public void OnProvidersExecuting_ThrowsIfControllerWithAttribute_HasActionsWithoutAttributeRouting()
         {
             // Arrange
+            var actionName = $"{typeof(ActionsWithoutAttributeRouting).FullName}.{nameof(ActionsWithoutAttributeRouting.Index)} ({typeof(ActionsWithoutAttributeRouting).Assembly.GetName().Name})";
+            var expected = $"Action '{actionName}' does not have an attribute route. Action methods on controllers annotated with ApiControllerAttribute must be attribute routed.";
             var context = GetContext(typeof(ActionsWithoutAttributeRouting));
             var provider = GetProvider();
 
             // Act & Assert
             var ex = Assert.Throws<InvalidOperationException>(() => provider.OnProvidersExecuting(context));
-            Assert.Equal(
+            Assert.Equal(expected, ex.Message);
-                "Action methods on controllers annotated with ApiControllerAttribute must have an attribute route.",
-                ex.Message);
         }
 
         [Fact]