Add helper methods on ControllerBase to return ProblemDetails

* Introduce ControllerBase.Problem and ValidationProblem overload
  that accepts optional parameters
* Consistently use ClientErrorData when generating ProblemDetails
* Clean-up InvalidModelStateResponseFactory initialization.

Fixes #8537

Author: pranavkm

src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs

@@ -445,6 +445,8 @@ protected ControllerBase() { }
         [Microsoft.AspNetCore.Mvc.NonActionAttribute]
         public virtual Microsoft.AspNetCore.Mvc.PhysicalFileResult PhysicalFile(string physicalPath, string contentType, string fileDownloadName, System.DateTimeOffset? lastModified, Microsoft.Net.Http.Headers.EntityTagHeaderValue entityTag, bool enableRangeProcessing) { throw null; }
         [Microsoft.AspNetCore.Mvc.NonActionAttribute]
+        public virtual Microsoft.AspNetCore.Mvc.ObjectResult Problem(string detail = null, string instance = null, string title = null, string type = null) { throw null; }
+        [Microsoft.AspNetCore.Mvc.NonActionAttribute]
         public virtual Microsoft.AspNetCore.Mvc.RedirectResult Redirect(string url) { throw null; }
         [Microsoft.AspNetCore.Mvc.NonActionAttribute]
         public virtual Microsoft.AspNetCore.Mvc.RedirectResult RedirectPermanent(string url) { throw null; }
@@ -586,6 +588,8 @@ protected ControllerBase() { }
         public virtual Microsoft.AspNetCore.Mvc.ActionResult ValidationProblem([Microsoft.AspNetCore.Mvc.Infrastructure.ActionResultObjectValueAttribute]Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary modelStateDictionary) { throw null; }
         [Microsoft.AspNetCore.Mvc.NonActionAttribute]
         public virtual Microsoft.AspNetCore.Mvc.ActionResult ValidationProblem([Microsoft.AspNetCore.Mvc.Infrastructure.ActionResultObjectValueAttribute]Microsoft.AspNetCore.Mvc.ValidationProblemDetails descriptor) { throw null; }
+        [Microsoft.AspNetCore.Mvc.NonActionAttribute]
+        public virtual Microsoft.AspNetCore.Mvc.ActionResult ValidationProblem(string detail, string instance = null, string title = null, string type = null, [Microsoft.AspNetCore.Mvc.Infrastructure.ActionResultObjectValueAttribute]Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary modelStateDictionary = null) { throw null; }
     }
     public partial class ControllerContext : Microsoft.AspNetCore.Mvc.ActionContext
     {

src/Mvc/Mvc.Core/src/ControllerBase.cs

@@ -16,6 +16,7 @@
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Options;
 using Microsoft.Net.Http.Headers;
 
 namespace Microsoft.AspNetCore.Mvc
@@ -1821,6 +1822,52 @@ public virtual ConflictObjectResult Conflict([ActionResultObjectValue] object er
         public virtual ConflictObjectResult Conflict([ActionResultObjectValue] ModelStateDictionary modelState)
             => new ConflictObjectResult(modelState);
 
+        /// <summary>
+        /// Creates an <see cref="ObjectResult"/> that produces a <see cref="ProblemDetails"/> response with a <c>500</c>
+        /// error status with a <see cref="ProblemDetails" /> value.
+        /// </summary>
+        /// <param name="title">The value for <see cref="ProblemDetails.Title" />.</param>
+        /// <param name="type">The value for <see cref="ProblemDetails.Type" />.</param>
+        /// <param name="detail">The value for <see cref="ProblemDetails.Detail" />.</param>
+        /// <param name="instance">The value for <see cref="ProblemDetails.Instance" />.</param>
+        /// <returns>The created <see cref="ObjectResult"/> for the response.</returns>
+        [NonAction]
+        public virtual ObjectResult Problem(
+            string detail = null,
+            string instance = null,
+            string title = null,
+            string type = null)
+        {
+            var problemDetails = new ProblemDetails
+            {
+                Title = title,
+                Type = type,
+                Detail = detail,
+                Instance = instance,
+            };
+
+            ApplyProblemDetailsDefaults(problemDetails, statusCode: 500);
+
+            return new ObjectResult(problemDetails);
+        }
+
+        private void ApplyProblemDetailsDefaults(ProblemDetails problemDetails, int statusCode)
+        {
+            problemDetails.Status = statusCode;
+
+            if (problemDetails.Title is null || problemDetails.Type is null)
+            {
+                var options = HttpContext.RequestServices.GetRequiredService<IOptions<ApiBehaviorOptions>>().Value;
+                if (options.ClientErrorMapping.TryGetValue(statusCode, out var clientErrorData))
+                {
+                    problemDetails.Title ??= clientErrorData.Title;
+                    problemDetails.Type ??= clientErrorData.Link;
+                }
+            }
+
+            ProblemDetailsClientErrorFactory.SetTraceId(ControllerContext, problemDetails);
+        }
+
         /// <summary>
         /// Creates an <see cref="BadRequestObjectResult"/> that produces a <see cref="StatusCodes.Status400BadRequest"/> response.
         /// </summary>
@@ -1837,8 +1884,10 @@ public virtual ActionResult ValidationProblem([ActionResultObjectValue] Validati
         }
 
         /// <summary>
-        /// Creates an <see cref="BadRequestObjectResult"/> that produces a <see cref="StatusCodes.Status400BadRequest"/> response.
+        /// Creates an <see cref="BadRequestObjectResult"/> that produces a <see cref="StatusCodes.Status400BadRequest"/> response
+        /// with validation errors from <paramref name="modelStateDictionary"/>.
         /// </summary>
+        /// <param name="modelStateDictionary">The <see cref="ModelStateDictionary"/>.</param>
         /// <returns>The created <see cref="BadRequestObjectResult"/> for the response.</returns>
         [NonAction]
         public virtual ActionResult ValidationProblem([ActionResultObjectValue] ModelStateDictionary modelStateDictionary)
@@ -1849,6 +1898,8 @@ public virtual ActionResult ValidationProblem([ActionResultObjectValue] ModelSta
             }
 
             var validationProblem = new ValidationProblemDetails(modelStateDictionary);
+            ApplyProblemDetailsDefaults(validationProblem, statusCode: 400);
+
             return new BadRequestObjectResult(validationProblem);
         }
 
@@ -1858,9 +1909,44 @@ public virtual ActionResult ValidationProblem([ActionResultObjectValue] ModelSta
         /// </summary>
         /// <returns>The created <see cref="BadRequestObjectResult"/> for the response.</returns>
         [NonAction]
-        public virtual ActionResult ValidationProblem()
+        public virtual ActionResult ValidationProblem() => ValidationProblem(ModelState);
+
+        /// <summary>
+        /// Creates an <see cref="BadRequestObjectResult"/> that produces a <see cref="StatusCodes.Status400BadRequest"/> response
+        /// with a <see cref="ValidationProblemDetails"/> value.
+        /// </summary>
+        /// <param name="title">The value for <see cref="ProblemDetails.Title" />.</param>
+        /// <param name="type">The value for <see cref="ProblemDetails.Type" />.</param>
+        /// <param name="detail">The value for <see cref="ProblemDetails.Detail" />.</param>
+        /// <param name="instance">The value for <see cref="ProblemDetails.Instance" />.</param>
+        /// <param name="modelStateDictionary">The <see cref="ModelStateDictionary"/>. 
+        /// When <see langword="null"/> uses <see cref="ModelState"/>.</param>
+        /// <returns>The created <see cref="BadRequestObjectResult"/> for the response.</returns>
+        [NonAction]
+        public virtual ActionResult ValidationProblem(
+            string detail,
+            string instance = null,
+            string title = null,
+            string type = null,
+            [ActionResultObjectValue] ModelStateDictionary modelStateDictionary = null)
         {
-            var validationProblem = new ValidationProblemDetails(ModelState);
+            modelStateDictionary ??= ModelState;
+
+            var validationProblem = new ValidationProblemDetails(modelStateDictionary)
+            {
+                Detail = detail,
+                Instance = instance,
+                Type = type,
+            };
+
+            if (title != null)
+            {
+                // ValidationProblemDetails has a Title by default. Do not overwrite it with a null
+                validationProblem.Title = title;
+            }
+
+            ApplyProblemDetailsDefaults(validationProblem, statusCode: 400);
+
             return new BadRequestObjectResult(validationProblem);
         }
 

src/Mvc/Mvc.Core/src/DependencyInjection/ApiBehaviorOptionsSetup.cs

@@ -10,34 +10,38 @@
 
 namespace Microsoft.Extensions.DependencyInjection
 {
-    internal class ApiBehaviorOptionsSetup :
-        IConfigureOptions<ApiBehaviorOptions>,
-        IPostConfigureOptions<ApiBehaviorOptions>
+    internal class ApiBehaviorOptionsSetup : IConfigureOptions<ApiBehaviorOptions>
     {
-        internal static readonly Func<ActionContext, IActionResult> DefaultFactory = DefaultInvalidModelStateResponse;
-        internal static readonly Func<ActionContext, IActionResult> ProblemDetailsFactory =
-            ProblemDetailsInvalidModelStateResponse;
-
         public void Configure(ApiBehaviorOptions options)
         {
             if (options == null)
             {
                 throw new ArgumentNullException(nameof(options));
             }
 
-            options.InvalidModelStateResponseFactory = DefaultFactory;
+            options.InvalidModelStateResponseFactory = ProblemDetailsInvalidModelStateResponse;
             ConfigureClientErrorMapping(options);
-        }
 
-        public void PostConfigure(string name, ApiBehaviorOptions options)
-        {
-            // We want to use problem details factory only if
-            // (a) it has not been opted out of (SuppressMapClientErrors = true)
-            // (b) a different factory was configured
-            if (!options.SuppressMapClientErrors &&
-                object.ReferenceEquals(options.InvalidModelStateResponseFactory, DefaultFactory))
+            IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context)
             {
-                options.InvalidModelStateResponseFactory = ProblemDetailsFactory;
+                var problemDetails = new ValidationProblemDetails(context.ModelState)
+                {
+                    Status = StatusCodes.Status400BadRequest,
+                };
+
+                if (options.ClientErrorMapping.TryGetValue(400, out var clientErrorData))
+                {
+                    problemDetails.Type = clientErrorData.Link;
+                }
+
+                ProblemDetailsClientErrorFactory.SetTraceId(context, problemDetails);
+
+                var result = new BadRequestObjectResult(problemDetails);
+
+                result.ContentTypes.Add("application/problem+json");
+                result.ContentTypes.Add("application/problem+xml");
+
+                return result;
             }
         }
 
@@ -91,33 +95,12 @@ internal static void ConfigureClientErrorMapping(ApiBehaviorOptions options)
                 Link = "https://tools.ietf.org/html/rfc4918#section-11.2",
                 Title = Resources.ApiConventions_Title_422,
             };
-        }
 
-        private static IActionResult DefaultInvalidModelStateResponse(ActionContext context)
-        {
-            var result = new BadRequestObjectResult(context.ModelState);
-
-            result.ContentTypes.Add("application/json");
-            result.ContentTypes.Add("application/xml");
-
-            return result;
-        }
-
-        internal static IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context)
-        {
-            var problemDetails = new ValidationProblemDetails(context.ModelState)
+            options.ClientErrorMapping[500] = new ClientErrorData
             {
-                Status = StatusCodes.Status400BadRequest,
+                Link = "https://tools.ietf.org/html/rfc7231#section-6.6.1",
+                Title = Resources.ApiConventions_Title_500,
             };
-
-            ProblemDetailsClientErrorFactory.SetTraceId(context, problemDetails);
-
-            var result = new BadRequestObjectResult(problemDetails);
-
-            result.ContentTypes.Add("application/problem+json");
-            result.ContentTypes.Add("application/problem+xml");
-
-            return result;
         }
     }
 }

src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

@@ -149,8 +149,6 @@ internal static void AddMvcCoreServices(IServiceCollection services)
                 ServiceDescriptor.Transient<IPostConfigureOptions<MvcOptions>, MvcCoreMvcOptionsSetup>());
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IConfigureOptions<ApiBehaviorOptions>, ApiBehaviorOptionsSetup>());
-            services.TryAddEnumerable(
-                ServiceDescriptor.Transient<IPostConfigureOptions<ApiBehaviorOptions>, ApiBehaviorOptionsSetup>());
             services.TryAddEnumerable(
                 ServiceDescriptor.Transient<IConfigureOptions<RouteOptions>, MvcCoreRouteOptionsSetup>());
 

src/Mvc/Mvc.Core/src/Resources.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!--
-    Microsoft ResX Schema
-
+  <!-- 
+    Microsoft ResX Schema 
+    
     Version 2.0
-
-    The primary goals of this format is to allow a simple XML format
-    that is mostly human readable. The generation and parsing of the
-    various data types are done through the TypeConverter classes
+    
+    The primary goals of this format is to allow a simple XML format 
+    that is mostly human readable. The generation and parsing of the 
+    various data types are done through the TypeConverter classes 
     associated with the data types.
-
+    
     Example:
-
+    
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-
-    There are any number of "resheader" rows that contain simple
+                
+    There are any number of "resheader" rows that contain simple 
     name/value pairs.
-
-    Each data row contains a name, and value. The row also contains a
-    type or mimetype. Type corresponds to a .NET class that support
-    text/value conversion through the TypeConverter architecture.
-    Classes that don't support this are serialized and stored with the
+    
+    Each data row contains a name, and value. The row also contains a 
+    type or mimetype. Type corresponds to a .NET class that support 
+    text/value conversion through the TypeConverter architecture. 
+    Classes that don't support this are serialized and stored with the 
     mimetype set.
-
-    The mimetype is used for serialized objects, and tells the
-    ResXResourceReader how to depersist the object. This is currently not
+    
+    The mimetype is used for serialized objects, and tells the 
+    ResXResourceReader how to depersist the object. This is currently not 
     extensible. For a given mimetype the value must be set accordingly:
-
-    Note - application/x-microsoft.net.object.binary.base64 is the format
-    that the ResXResourceWriter will generate, however the reader can
+    
+    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    that the ResXResourceWriter will generate, however the reader can 
     read any of the formats listed below.
-
+    
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-
+    
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with
+    value   : The object must be serialized with 
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array
+    value   : The object must be serialized into a byte array 
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -507,4 +507,7 @@
   <data name="ObjectResultExecutor_MaxEnumerationExceeded" xml:space="preserve">
     <value>'{0}' reached the configured maximum size of the buffer when enumerating a value of type '{1}'. This limit is in place to prevent infinite streams of 'IAsyncEnumerable&lt;&gt;' from continuing indefinitely. If this is not a programming mistake, consider ways to reduce the collection size, or consider manually converting '{1}' into a list rather than increasing the limit.</value>
   </data>
-</root>
+  <data name="ApiConventions_Title_500" xml:space="preserve">
+    <value>An error occured while processing your request.</value>
+  </data>
+</root>