Adding ProblemDetailsService (#42384)

* MVC Changes * ExceptionHandler changes * Routing changes * Http.Extensions changes * Minimal APi draft changes * HttpResults changes * New ProblemDetails project * Using ProblemDetailsOptions in mvc * ProblemDetails project simplification * Using metadata * Using metadata * Latest version * Removing changes * Initial cleanup * Clean up * Public api clean up * Updating public API * More clean ups * Adding initial unit tests * Updating unit tests * Adding more unit tests * Cleanup * Clean up * clean up * clean up * Removing nullable * Simplifying public api * API Review feedback * API review feedback * Clean up * Clean up * clean up * Reusing Endpoint & EndpointMetadataCollection * Fix build issues * Updates based on docs/Trimming.md * Adding Functional tests * Fix unittest * Seal context * Seal ProblemMetadata * PR Feeback * API Review * Clean up * Fixing publicapi warnings * Clean up * PR Feedback * Fixing build * Fix unit test * Fix unit tests * PR review * Fixing JsonSerializationContext issues * Adding statuscode 405 * Update src/Http/Http.Extensions/src/ProblemDetailsDefaultWriter.cs Co-authored-by: Brennan <brecon@microsoft.com> * PR review * Apply suggestions from code review Co-authored-by: Stephen Halter <halter73@gmail.com> * Fixing bad merge * Fix bad merge * Adding analysis.NextMiddlewareName * PR review * Changing to CanWrite/WriteAsync Co-authored-by: Brennan <brecon@microsoft.com> Co-authored-by: Stephen Halter <halter73@gmail.com>
dotnet · Jul 14, 2022 · ac67d32 · ac67d32
1 parent ca77aff
commit ac67d32
Show file tree

Hide file tree

Showing 69 changed files with 1,968 additions and 514 deletions.
diff --git a/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj b/src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj
@@ -26,6 +26,8 @@ Microsoft.AspNetCore.Http.HttpResponse</Description>
     <Compile Include="$(SharedSourceRoot)PropertyHelper\**\*.cs" />
     <Compile Include="$(SharedSourceRoot)\UrlDecoder\UrlDecoder.cs" Link="UrlDecoder.cs" />
     <Compile Include="$(SharedSourceRoot)ValueTaskExtensions\**\*.cs" />
+    <Compile Include="$(SharedSourceRoot)ProblemDetails\HttpValidationProblemDetailsJsonConverter.cs" LinkBase="ProblemDetails\Converters" />
+    <Compile Include="$(SharedSourceRoot)ProblemDetails\ProblemDetailsJsonConverter.cs" LinkBase="ProblemDetails\Converters" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/...sions/src/HttpValidationProblemDetails.cs → ...emDetails/HttpValidationProblemDetails.cs b/...sions/src/HttpValidationProblemDetails.cs → ...emDetails/HttpValidationProblemDetails.cs
diff --git a/src/Http/Http.Abstractions/src/ProblemDetails/IProblemDetailsService.cs b/src/Http/Http.Abstractions/src/ProblemDetails/IProblemDetailsService.cs
@@ -0,0 +1,23 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Http;
+
+/// <summary>
+/// Defines a type that provide functionality to
+/// create a <see cref="Mvc.ProblemDetails"/> response.
+/// </summary>
+public interface IProblemDetailsService
+{
+    /// <summary>
+    /// Try to write a <see cref="Mvc.ProblemDetails"/> response to the current context,
+    /// using the registered <see cref="IProblemDetailsWriter"/> services.
+    /// </summary>
+    /// <param name="context">The <see cref="ProblemDetailsContext"/> associated with the current request/response.</param>
+    /// <remarks>The <see cref="IProblemDetailsWriter"/> registered services
+    /// are processed in sequence and the processing is completed when:
+    /// <list type="bullet">One of them reports that the response was written successfully, or.</list>
+    /// <list type="bullet">All <see cref="IProblemDetailsWriter"/> were executed and none of them was able to write the response successfully.</list>
+    /// </remarks>
+    ValueTask WriteAsync(ProblemDetailsContext context);
+}
diff --git a/src/Http/Http.Abstractions/src/ProblemDetails/IProblemDetailsWriter.cs b/src/Http/Http.Abstractions/src/ProblemDetails/IProblemDetailsWriter.cs
@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Http;
+
+/// <summary>
+/// Defines a type that write a <see cref="Mvc.ProblemDetails"/>
+/// payload to the current <see cref="HttpContext.Response"/>.
+/// </summary>
+public interface IProblemDetailsWriter
+{
+    /// <summary>
+    /// Write a <see cref="Mvc.ProblemDetails"/> response to the current context
+    /// </summary>
+    /// <param name="context">The <see cref="ProblemDetailsContext"/> associated with the current request/response.</param>
+    ValueTask WriteAsync(ProblemDetailsContext context);
+
+    /// <summary>
+    /// Determines whether this instance can write a <see cref="Mvc.ProblemDetails"/> to the current context.
+    /// </summary>
+    /// <param name="context">The <see cref="ProblemDetailsContext"/> associated with the current request/response.</param>
+    /// <returns>Flag that indicates if that the writer can write to the current <see cref="ProblemDetailsContext"/>.</returns>
+    bool CanWrite(ProblemDetailsContext context);
+}
diff --git a/...ttp/Http.Extensions/src/ProblemDetails.cs → ...ions/src/ProblemDetails/ProblemDetails.cs b/...ttp/Http.Extensions/src/ProblemDetails.cs → ...ions/src/ProblemDetails/ProblemDetails.cs
diff --git a/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetailsContext.cs b/src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetailsContext.cs
@@ -0,0 +1,34 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Mvc;
+
+namespace Microsoft.AspNetCore.Http;
+
+/// <summary>
+/// Represent the current problem details context for the request.
+/// </summary>
+public sealed class ProblemDetailsContext
+{
+    private ProblemDetails? _problemDetails;
+
+    /// <summary>
+    /// The <see cref="HttpContext"/> associated with the current request being processed by the filter.
+    /// </summary>
+    public required HttpContext HttpContext { get; init; }
+
+    /// <summary>
+    /// A collection of additional arbitrary metadata associated with the current request endpoint.
+    /// </summary>
+    public EndpointMetadataCollection? AdditionalMetadata { get; init; }
+
+    /// <summary>
+    /// An instance of <see cref="ProblemDetails"/> that will be
+    /// used during the response payload generation.
+    /// </summary>
+    public ProblemDetails ProblemDetails
+    {
+        get => _problemDetails ??= new ProblemDetails();
+        init => _problemDetails = value;
+    }
+}
diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt
@@ -21,6 +21,10 @@ Microsoft.AspNetCore.Http.EndpointFilterInvocationContext
 Microsoft.AspNetCore.Http.EndpointFilterInvocationContext.EndpointFilterInvocationContext() -> void
 Microsoft.AspNetCore.Http.EndpointMetadataCollection.Enumerator.Current.get -> object!
 Microsoft.AspNetCore.Http.EndpointMetadataCollection.GetRequiredMetadata<T>() -> T!
+Microsoft.AspNetCore.Http.HttpValidationProblemDetails
+Microsoft.AspNetCore.Http.HttpValidationProblemDetails.Errors.get -> System.Collections.Generic.IDictionary<string!, string![]!>!
+Microsoft.AspNetCore.Http.HttpValidationProblemDetails.HttpValidationProblemDetails() -> void
+Microsoft.AspNetCore.Http.HttpValidationProblemDetails.HttpValidationProblemDetails(System.Collections.Generic.IDictionary<string!, string![]!>! errors) -> void
 Microsoft.AspNetCore.Http.IBindableFromHttpContext<TSelf>
 Microsoft.AspNetCore.Http.IBindableFromHttpContext<TSelf>.BindAsync(Microsoft.AspNetCore.Http.HttpContext! context, System.Reflection.ParameterInfo! parameter) -> System.Threading.Tasks.ValueTask<TSelf?>
 Microsoft.AspNetCore.Http.IContentTypeHttpResult
@@ -30,8 +34,13 @@ Microsoft.AspNetCore.Http.IEndpointFilter.InvokeAsync(Microsoft.AspNetCore.Http.
 Microsoft.AspNetCore.Http.IFileHttpResult
 Microsoft.AspNetCore.Http.IFileHttpResult.ContentType.get -> string?
 Microsoft.AspNetCore.Http.IFileHttpResult.FileDownloadName.get -> string?
+Microsoft.AspNetCore.Http.IProblemDetailsService
+Microsoft.AspNetCore.Http.IProblemDetailsService.WriteAsync(Microsoft.AspNetCore.Http.ProblemDetailsContext! context) -> System.Threading.Tasks.ValueTask
+Microsoft.AspNetCore.Http.IProblemDetailsWriter
 Microsoft.AspNetCore.Http.INestedHttpResult
 Microsoft.AspNetCore.Http.INestedHttpResult.Result.get -> Microsoft.AspNetCore.Http.IResult!
+Microsoft.AspNetCore.Http.IProblemDetailsWriter.CanWrite(Microsoft.AspNetCore.Http.ProblemDetailsContext! context) -> bool
+Microsoft.AspNetCore.Http.IProblemDetailsWriter.WriteAsync(Microsoft.AspNetCore.Http.ProblemDetailsContext! context) -> System.Threading.Tasks.ValueTask
 Microsoft.AspNetCore.Http.IStatusCodeHttpResult
 Microsoft.AspNetCore.Http.IStatusCodeHttpResult.StatusCode.get -> int?
 Microsoft.AspNetCore.Http.IValueHttpResult
@@ -42,6 +51,14 @@ Microsoft.AspNetCore.Http.Metadata.IFromFormMetadata
 Microsoft.AspNetCore.Http.Metadata.IFromFormMetadata.Name.get -> string?
 Microsoft.AspNetCore.Http.Metadata.IRequestSizeLimitMetadata
 Microsoft.AspNetCore.Http.Metadata.IRequestSizeLimitMetadata.MaxRequestBodySize.get -> long?
+Microsoft.AspNetCore.Http.ProblemDetailsContext
+Microsoft.AspNetCore.Http.ProblemDetailsContext.AdditionalMetadata.get -> Microsoft.AspNetCore.Http.EndpointMetadataCollection?
+Microsoft.AspNetCore.Http.ProblemDetailsContext.AdditionalMetadata.init -> void
+Microsoft.AspNetCore.Http.ProblemDetailsContext.HttpContext.get -> Microsoft.AspNetCore.Http.HttpContext!
+Microsoft.AspNetCore.Http.ProblemDetailsContext.HttpContext.init -> void
+Microsoft.AspNetCore.Http.ProblemDetailsContext.ProblemDetails.get -> Microsoft.AspNetCore.Mvc.ProblemDetails!
+Microsoft.AspNetCore.Http.ProblemDetailsContext.ProblemDetails.init -> void
+Microsoft.AspNetCore.Http.ProblemDetailsContext.ProblemDetailsContext() -> void
 Microsoft.AspNetCore.Routing.RouteValueDictionary.RouteValueDictionary(Microsoft.AspNetCore.Routing.RouteValueDictionary? dictionary) -> void
 Microsoft.AspNetCore.Routing.RouteValueDictionary.RouteValueDictionary(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, object?>>? values) -> void
 Microsoft.AspNetCore.Routing.RouteValueDictionary.RouteValueDictionary(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string!, string?>>? values) -> void
@@ -51,6 +68,19 @@ Microsoft.AspNetCore.Http.Metadata.IEndpointDescriptionMetadata
 Microsoft.AspNetCore.Http.Metadata.IEndpointDescriptionMetadata.Description.get -> string!
 Microsoft.AspNetCore.Http.Metadata.IEndpointSummaryMetadata
 Microsoft.AspNetCore.Http.Metadata.IEndpointSummaryMetadata.Summary.get -> string!
+Microsoft.AspNetCore.Mvc.ProblemDetails
+Microsoft.AspNetCore.Mvc.ProblemDetails.Detail.get -> string?
+Microsoft.AspNetCore.Mvc.ProblemDetails.Detail.set -> void
+Microsoft.AspNetCore.Mvc.ProblemDetails.Extensions.get -> System.Collections.Generic.IDictionary<string!, object?>!
+Microsoft.AspNetCore.Mvc.ProblemDetails.Instance.get -> string?
+Microsoft.AspNetCore.Mvc.ProblemDetails.Instance.set -> void
+Microsoft.AspNetCore.Mvc.ProblemDetails.ProblemDetails() -> void
+Microsoft.AspNetCore.Mvc.ProblemDetails.Status.get -> int?
+Microsoft.AspNetCore.Mvc.ProblemDetails.Status.set -> void
+Microsoft.AspNetCore.Mvc.ProblemDetails.Title.get -> string?
+Microsoft.AspNetCore.Mvc.ProblemDetails.Title.set -> void
+Microsoft.AspNetCore.Mvc.ProblemDetails.Type.get -> string?
+Microsoft.AspNetCore.Mvc.ProblemDetails.Type.set -> void
 override Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.Arguments.get -> System.Collections.Generic.IList<object?>!
 override Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.GetArgument<T>(int index) -> T
 override Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.HttpContext.get -> Microsoft.AspNetCore.Http.HttpContext!
diff --git a/...idationProblemDetailsJsonConverterTest.cs → ...idationProblemDetailsJsonConverterTest.cs b/...idationProblemDetailsJsonConverterTest.cs → ...idationProblemDetailsJsonConverterTest.cs
@@ -5,7 +5,7 @@
 using System.Text.Json;
 using Microsoft.AspNetCore.Http.Json;
 
-namespace Microsoft.AspNetCore.Http.Extensions;
+namespace Microsoft.AspNetCore.Http.Abstractions.Tests;
 
 public class HttpValidationProblemDetailsJsonConverterTest
 {
@@ -40,7 +40,7 @@ public void Read_Works()
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
         Assert.Collection(
             problemDetails.Errors.OrderBy(kvp => kvp.Key),
@@ -81,7 +81,7 @@ public void Read_WithSomeMissingValues_Works()
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
         Assert.Collection(
             problemDetails.Errors.OrderBy(kvp => kvp.Key),
@@ -111,15 +111,16 @@ public void ReadUsingJsonSerializerWorks()
         // Act
         var problemDetails = JsonSerializer.Deserialize<HttpValidationProblemDetails>(json, JsonSerializerOptions);
 
-        Assert.Equal(type, problemDetails.Type);
+        Assert.NotNull(problemDetails);
+        Assert.Equal(type, problemDetails!.Type);
         Assert.Equal(title, problemDetails.Title);
         Assert.Equal(status, problemDetails.Status);
         Assert.Collection(
             problemDetails.Extensions,
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
         Assert.Collection(
             problemDetails.Errors.OrderBy(kvp => kvp.Key),

diff --git a/...s/test/ProblemDetailsJsonConverterTest.cs → ...s/test/ProblemDetailsJsonConverterTest.cs b/...s/test/ProblemDetailsJsonConverterTest.cs → ...s/test/ProblemDetailsJsonConverterTest.cs
@@ -6,7 +6,7 @@
 using Microsoft.AspNetCore.Http.Json;
 using Microsoft.AspNetCore.Mvc;
 
-namespace Microsoft.AspNetCore.Http.Extensions;
+namespace Microsoft.AspNetCore.Http.Abstractions.Tests;
 
 public class ProblemDetailsJsonConverterTest
 {
@@ -46,6 +46,7 @@ public void Read_Works()
         // Act
         var problemDetails = converter.Read(ref reader, typeof(ProblemDetails), JsonSerializerOptions);
 
+        //Assert
         Assert.Equal(type, problemDetails.Type);
         Assert.Equal(title, problemDetails.Title);
         Assert.Equal(status, problemDetails.Status);
@@ -56,7 +57,7 @@ public void Read_Works()
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
     }
 
@@ -75,7 +76,9 @@ public void Read_UsingJsonSerializerWorks()
         // Act
         var problemDetails = JsonSerializer.Deserialize<ProblemDetails>(json, JsonSerializerOptions);
 
-        Assert.Equal(type, problemDetails.Type);
+        // Assert
+        Assert.NotNull(problemDetails);
+        Assert.Equal(type, problemDetails!.Type);
         Assert.Equal(title, problemDetails.Title);
         Assert.Equal(status, problemDetails.Status);
         Assert.Equal(instance, problemDetails.Instance);
@@ -85,7 +88,7 @@ public void Read_UsingJsonSerializerWorks()
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
     }
 
@@ -105,6 +108,7 @@ public void Read_WithSomeMissingValues_Works()
         // Act
         var problemDetails = converter.Read(ref reader, typeof(ProblemDetails), JsonSerializerOptions);
 
+        // Assert
         Assert.Equal(type, problemDetails.Type);
         Assert.Equal(title, problemDetails.Title);
         Assert.Equal(status, problemDetails.Status);
@@ -113,7 +117,7 @@ public void Read_WithSomeMissingValues_Works()
             kvp =>
             {
                 Assert.Equal("traceId", kvp.Key);
-                Assert.Equal(traceId, kvp.Value.ToString());
+                Assert.Equal(traceId, kvp.Value?.ToString());
             });
     }
 

diff --git a/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs b/src/Http/Http.Extensions/src/DefaultProblemDetailsWriter.cs
@@ -0,0 +1,64 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics.CodeAnalysis;
+using System.Linq;
+using System.Text.Json.Serialization;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Extensions.Options;
+using Microsoft.Net.Http.Headers;
+
+namespace Microsoft.AspNetCore.Http;
+
+internal sealed partial class DefaultProblemDetailsWriter : IProblemDetailsWriter
+{
+    private static readonly MediaTypeHeaderValue _jsonMediaType = new("application/json");
+    private static readonly MediaTypeHeaderValue _problemDetailsJsonMediaType = new("application/problem+json");
+    private readonly ProblemDetailsOptions _options;
+
+    public DefaultProblemDetailsWriter(IOptions<ProblemDetailsOptions> options)
+    {
+        _options = options.Value;
+    }
+
+    public bool CanWrite(ProblemDetailsContext context)
+    {
+        var httpContext = context.HttpContext;
+        var acceptHeader = httpContext.Request.Headers.Accept.GetList<MediaTypeHeaderValue>();
+
+        if (acceptHeader?.Any(h => _jsonMediaType.IsSubsetOf(h) || _problemDetailsJsonMediaType.IsSubsetOf(h)) == true)
+        {
+            return true;
+        }
+
+        return false;
+    }
+
+    [UnconditionalSuppressMessage("Trimming", "IL2026",
+        Justification = "JSON serialization of ProblemDetails.Extensions might require types that cannot be statically analyzed and we need to fallback" +
+        "to reflection-based. The ProblemDetailsConverter is marked as RequiresUnreferencedCode already.")]
+    public ValueTask WriteAsync(ProblemDetailsContext context)
+    {
+        var httpContext = context.HttpContext;
+        ProblemDetailsDefaults.Apply(context.ProblemDetails, httpContext.Response.StatusCode);
+        _options.CustomizeProblemDetails?.Invoke(context);
+
+        if (context.ProblemDetails.Extensions is { Count: 0 })
+        {
+            // We can use the source generation in this case
+            return new ValueTask(httpContext.Response.WriteAsJsonAsync(
+                context.ProblemDetails,
+                ProblemDetailsJsonContext.Default.ProblemDetails,
+                contentType: "application/problem+json"));
+        }
+
+        return new ValueTask(httpContext.Response.WriteAsJsonAsync(
+                        context.ProblemDetails,
+                        options: null,
+                        contentType: "application/problem+json"));
+    }
+
+    [JsonSerializable(typeof(ProblemDetails))]
+    internal sealed partial class ProblemDetailsJsonContext : JsonSerializerContext
+    { }
+}
diff --git a/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj b/src/Http/Http.Extensions/src/Microsoft.AspNetCore.Http.Extensions.csproj
@@ -15,10 +15,9 @@
     <Compile Include="$(SharedSourceRoot)ParameterBindingMethodCache.cs" LinkBase="Shared"/>
     <Compile Include="$(SharedSourceRoot)PropertyAsParameterInfo.cs" LinkBase="Shared"/>
     <Compile Include="..\..\Shared\StreamCopyOperationInternal.cs" LinkBase="Shared" />
-    <Compile Include="$(SharedSourceRoot)ProblemDetailsJsonConverter.cs" LinkBase="Shared"/>
-    <Compile Include="$(SharedSourceRoot)HttpValidationProblemDetailsJsonConverter.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)RoutingMetadata\AcceptsMetadata.cs" LinkBase="Shared" />
     <Compile Include="$(SharedSourceRoot)TypeNameHelper\TypeNameHelper.cs" LinkBase="Shared"/>
+    <Compile Include="$(SharedSourceRoot)ProblemDetails\ProblemDetailsDefaults.cs" LinkBase="Shared" />
   </ItemGroup>
 
   <ItemGroup>

diff --git a/src/Http/Http.Extensions/src/ProblemDetailsOptions.cs b/src/Http/Http.Extensions/src/ProblemDetailsOptions.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Http;
+
+/// <summary>
+/// Options for controlling the behavior of <see cref="IProblemDetailsService.WriteAsync(ProblemDetailsContext)"/>
+/// and similar methods.
+/// </summary>
+public class ProblemDetailsOptions
+{
+    /// <summary>
+    /// The operation that customizes the current <see cref="Mvc.ProblemDetails"/> instance.
+    /// </summary>
+    public Action<ProblemDetailsContext>? CustomizeProblemDetails { get; set; }
+}