Add Description to ProducesResponseType (and others) for better OpenAPI document creation (dotnet#58193)

sander1095 · web-flow · commit d7130a2c5a99 · 2024-10-29T11:24:52.000-07:00
* Started adding Description to several attributes

* Add new properties to unshipped apis

* Some more progress of adding Description in some places

* Small improvements based on API review comments

* Added missing modifier to property

* Make changes in unshipped.txt so the http project builds

* Add Description to OpenApiRouteHandlerBuilderExtensions and extra constructor and static methods to ProducesResponseTypeMetadata for correct description overloads

* Changed code to follow overload rules and fix compile issues

* Fix minor typo

* Remove code from OpenApiRouteHandlerBUilderExtensions based on Safiaś comment

Also removed new constructors based on Safiaś comment

* Fix incorrect XML Comment

* Fixed some more Public API issues

* Add unit test

* Add some more unit tests

* Remove unnecessary set from interface

* Remove unnecessary change in public api shipped file

* Also update unshipped file so the build succeeds!

* Apply the new Description in response models
diff --git a/src/Http/Http.Abstractions/src/Metadata/IProducesResponseTypeMetadata.cs b/src/Http/Http.Abstractions/src/Metadata/IProducesResponseTypeMetadata.cs
@@ -18,6 +18,11 @@ public interface IProducesResponseTypeMetadata
     /// </summary>
     int StatusCode { get; }
 
+    /// <summary>
+    /// Gets the description of the response.
+    /// </summary>
+    string? Description { get; }
+
     /// <summary>
     /// Gets the content types supported by the metadata.
     /// </summary>
diff --git a/src/Http/Http.Abstractions/src/Metadata/ProducesResponseTypeMetadata.cs b/src/Http/Http.Abstractions/src/Metadata/ProducesResponseTypeMetadata.cs
@@ -68,6 +68,11 @@ private ProducesResponseTypeMetadata(int statusCode, Type? type, IEnumerable<str
     /// </summary>
     public int StatusCode { get; private set; }
 
+    /// <summary>
+    /// Gets or sets the description of the response.
+    /// </summary>
+    public string? Description { get; set; }
+
     /// <summary>
     /// Gets or sets the content types associated with the response.
     /// </summary>
diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt
@@ -11,3 +11,6 @@ Microsoft.AspNetCore.Http.Metadata.IParameterBindingMetadata.HasTryParse.get ->
 Microsoft.AspNetCore.Http.Metadata.IParameterBindingMetadata.IsOptional.get -> bool
 Microsoft.AspNetCore.Http.Metadata.IParameterBindingMetadata.Name.get -> string!
 Microsoft.AspNetCore.Http.Metadata.IParameterBindingMetadata.ParameterInfo.get -> System.Reflection.ParameterInfo!
+Microsoft.AspNetCore.Http.ProducesResponseTypeMetadata.Description.get -> string?
+Microsoft.AspNetCore.Http.ProducesResponseTypeMetadata.Description.set -> void
+Microsoft.AspNetCore.Http.Metadata.IProducesResponseTypeMetadata.Description.get -> string?
diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs
@@ -274,7 +274,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat
         var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance;
         var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider);
         var jsonSerializerOptions = serviceProvider.GetService<IOptions<JsonOptions>>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions;
-        var formDataMapperOptions = new FormDataMapperOptions();;
+        var formDataMapperOptions = new FormDataMapperOptions();
 
         var factoryContext = new RequestDelegateFactoryContext
         {
diff --git a/src/Mvc/Mvc.Abstractions/src/ApiExplorer/ApiResponseType.cs b/src/Mvc/Mvc.Abstractions/src/ApiExplorer/ApiResponseType.cs
@@ -33,6 +33,11 @@ public class ApiResponseType
     /// </remarks>
     public Type? Type { get; set; }
 
+    /// <summary>
+    /// Gets or sets the description of the response.
+    /// </summary>
+    public string? Description { get; set; }
+
     /// <summary>
     /// Gets or sets the HTTP response status code.
     /// </summary>
diff --git a/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt
@@ -1 +1,3 @@
 #nullable enable
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.get -> string?
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.set -> void
diff --git a/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs b/src/Mvc/Mvc.Api.Analyzers/test/TestFiles/SymbolApiResponseMetadataProviderTest/GetResponseMetadataTests.cs
@@ -1,4 +1,4 @@
-﻿// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
@@ -60,6 +60,8 @@ public class CustomResponseTypeAttribute : Attribute, IApiResponseMetadataProvid
 
         public int StatusCode { get; set; }
 
+        public string Description { get; set; }
+
         public void SetContentTypes(MediaTypeCollection contentTypes)
         {
         }
diff --git a/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/ApiResponseTypeProvider.cs
@@ -166,11 +166,14 @@ internal static Dictionary<int, ApiResponseType> ReadResponseMetadata(
 
                 var statusCode = metadataAttribute.StatusCode;
 
+                var description = metadataAttribute.Description;
+
                 var apiResponseType = new ApiResponseType
                 {
                     Type = metadataAttribute.Type,
                     StatusCode = statusCode,
                     IsDefaultResponse = metadataAttribute is IApiDefaultResponseMetadataProvider,
+                    Description = description
                 };
 
                 if (apiResponseType.Type == typeof(void))
diff --git a/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt
@@ -1 +1,3 @@
 #nullable enable
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.get -> string? (forwarded, contained in Microsoft.AspNetCore.Mvc.Abstractions)
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.set -> void (forwarded, contained in Microsoft.AspNetCore.Mvc.Abstractions)
diff --git a/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/DefaultApiDescriptionProviderTest.cs
@@ -1107,7 +1107,8 @@ public void GetApiDescription_PopulatesResponseInformation_WhenSetByFilter(strin
         var action = CreateActionDescriptor(methodName);
         var filter = new ContentTypeAttribute("text/*")
         {
-            Type = typeof(Order)
+            Type = typeof(Order),
+            Description = "Example"
         };
 
         action.FilterDescriptors = new List<FilterDescriptor>
@@ -1124,6 +1125,7 @@ public void GetApiDescription_PopulatesResponseInformation_WhenSetByFilter(strin
         Assert.NotNull(responseTypes.ModelMetadata);
         Assert.Equal(200, responseTypes.StatusCode);
         Assert.Equal(typeof(Order), responseTypes.Type);
+        Assert.Equal("Example", responseTypes.Description);
 
         foreach (var responseFormat in responseTypes.ApiResponseFormats)
         {
@@ -2874,6 +2876,8 @@ public ContentTypeAttribute(string mediaType)
 
         public Type Type { get; set; }
 
+        public string Description { get; set; }
+
         public void SetContentTypes(MediaTypeCollection contentTypes)
         {
             contentTypes.Clear();
diff --git a/src/Mvc/Mvc.Core/src/ApiExplorer/IApiResponseMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ApiExplorer/IApiResponseMetadataProvider.cs
@@ -17,6 +17,11 @@ public interface IApiResponseMetadataProvider : IFilterMetadata
     /// </summary>
     Type? Type { get; }
 
+    /// <summary>
+    /// Gets the description of the response.
+    /// </summary>
+    string? Description { get; }
+
     /// <summary>
     /// Gets the HTTP status code of the response.
     /// </summary>
diff --git a/src/Mvc/Mvc.Core/src/ProducesAttribute.cs b/src/Mvc/Mvc.Core/src/ProducesAttribute.cs
@@ -52,6 +52,9 @@ public ProducesAttribute(string contentType, params string[] additionalContentTy
     /// <inheritdoc />
     public Type? Type { get; set; }
 
+    /// <inheritdoc />
+    public string? Description { get; set; }
+
     /// <summary>
     /// Gets or sets the supported response content types. Used to set <see cref="ObjectResult.ContentTypes"/>.
     /// </summary>
diff --git a/src/Mvc/Mvc.Core/src/ProducesDefaultResponseTypeAttribute.cs b/src/Mvc/Mvc.Core/src/ProducesDefaultResponseTypeAttribute.cs
@@ -1,4 +1,4 @@
-﻿// Licensed to the .NET Foundation under one or more agreements.
+// 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.ApiExplorer;
@@ -39,6 +39,11 @@ public ProducesDefaultResponseTypeAttribute(Type type)
     /// </summary>
     public int StatusCode { get; }
 
+    /// <summary>
+    /// Gets or sets the description of the response.
+    /// </summary>
+    public string? Description { get; set; }
+
     /// <inheritdoc />
     void IApiResponseMetadataProvider.SetContentTypes(MediaTypeCollection contentTypes)
     {
diff --git a/src/Mvc/Mvc.Core/src/ProducesResponseTypeAttribute.cs b/src/Mvc/Mvc.Core/src/ProducesResponseTypeAttribute.cs
@@ -1,4 +1,4 @@
-﻿// Licensed to the .NET Foundation under one or more agreements.
+// 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.ApiExplorer;
@@ -87,6 +87,11 @@ public ProducesResponseTypeAttribute(Type type, int statusCode, string contentTy
     // Internal for testing
     internal MediaTypeCollection? ContentTypes => _contentTypes;
 
+    /// <summary>
+    /// Gets or sets the description of the response.
+    /// </summary>
+    public string? Description { get; set; }
+
     /// <inheritdoc />
     void IApiResponseMetadataProvider.SetContentTypes(MediaTypeCollection contentTypes)
     {
diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt
@@ -1,6 +1,13 @@
 #nullable enable
 *REMOVED*virtual Microsoft.AspNetCore.Mvc.ControllerBase.Problem(string? detail = null, string? instance = null, int? statusCode = null, string? title = null, string? type = null) -> Microsoft.AspNetCore.Mvc.ObjectResult!
 *REMOVED*virtual Microsoft.AspNetCore.Mvc.ControllerBase.ValidationProblem(string? detail = null, string? instance = null, int? statusCode = null, string? title = null, string? type = null, Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary? modelStateDictionary = null) -> Microsoft.AspNetCore.Mvc.ActionResult!
+Microsoft.AspNetCore.Mvc.ApiExplorer.IApiResponseMetadataProvider.Description.get -> string?
+Microsoft.AspNetCore.Mvc.ProducesAttribute.Description.get -> string?
+Microsoft.AspNetCore.Mvc.ProducesAttribute.Description.set -> void
+Microsoft.AspNetCore.Mvc.ProducesDefaultResponseTypeAttribute.Description.get -> string?
+Microsoft.AspNetCore.Mvc.ProducesDefaultResponseTypeAttribute.Description.set -> void
+Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute.Description.get -> string?
+Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute.Description.set -> void
 virtual Microsoft.AspNetCore.Mvc.ControllerBase.Problem(string? detail, string? instance, int? statusCode, string? title, string? type) -> Microsoft.AspNetCore.Mvc.ObjectResult!
 virtual Microsoft.AspNetCore.Mvc.ControllerBase.Problem(string? detail = null, string? instance = null, int? statusCode = null, string? title = null, string? type = null, System.Collections.Generic.IDictionary<string!, object?>? extensions = null) -> Microsoft.AspNetCore.Mvc.ObjectResult!
 virtual Microsoft.AspNetCore.Mvc.ControllerBase.ValidationProblem(string? detail, string? instance, int? statusCode, string? title, string? type, Microsoft.AspNetCore.Mvc.ModelBinding.ModelStateDictionary? modelStateDictionary) -> Microsoft.AspNetCore.Mvc.ActionResult!
diff --git a/src/Mvc/Mvc.Core/test/ProducesAttributeTests.cs b/src/Mvc/Mvc.Core/test/ProducesAttributeTests.cs
@@ -151,6 +151,29 @@ public void ProducesAttribute_WithTypeOnly_DoesNotSetContentTypes()
         Assert.Empty(producesAttribute.ContentTypes);
     }
 
+    [Fact]
+    public void ProducesAttribute_SetsDescription()
+    {
+        // Arrange
+        var producesAttribute = new ProducesAttribute(typeof(Person))
+        {
+            Description = "Example"
+        };
+
+        // Act and Assert
+        Assert.Equal("Example", producesAttribute.Description);
+    }
+
+    [Fact]
+    public void ProducesAttribute_WithTypeOnly_DoesNotSetDescription()
+    {
+        // Arrange
+        var producesAttribute = new ProducesAttribute(typeof(Person));
+
+        // Act and Assert
+        Assert.Null(producesAttribute.Description);
+    }
+
     private static ResultExecutedContext CreateResultExecutedContext(ResultExecutingContext context)
     {
         return new ResultExecutedContext(context, context.Filters, context.Result, context.Controller);
diff --git a/src/Mvc/Mvc.Core/test/ProducesResponseTypeAttributeTests.cs b/src/Mvc/Mvc.Core/test/ProducesResponseTypeAttributeTests.cs
@@ -67,6 +67,29 @@ public void ProducesResponseTypeAttribute_WithTypeOnly_DoesNotSetContentTypes()
         Assert.Null(producesResponseTypeAttribute.ContentTypes);
     }
 
+    [Fact]
+    public void ProducesResponseTypeAttribute_SetsDescription()
+    {
+        // Arrange
+        var producesResponseTypeAttribute = new ProducesResponseTypeAttribute(typeof(Person), StatusCodes.Status200OK)
+        {
+            Description = "Example"
+        };
+
+        // Act and Assert
+        Assert.Equal("Example", producesResponseTypeAttribute.Description);
+    }
+
+    [Fact]
+    public void ProducesResponseTypeAttribute_WithTypeOnly_DoesNotSetDescription()
+    {
+        // Arrange
+        var producesResponseTypeAttribute = new ProducesResponseTypeAttribute(typeof(Person), StatusCodes.Status200OK);
+
+        // Act and Assert
+        Assert.Null(producesResponseTypeAttribute.Description);
+    }
+
     private class Person
     {
         public int Id { get; set; }
diff --git a/src/OpenApi/src/Services/OpenApiDocumentService.cs b/src/OpenApi/src/Services/OpenApiDocumentService.cs
@@ -356,10 +356,9 @@ private async Task<OpenApiResponse> GetResponseAsync(
         IOpenApiSchemaTransformer[] schemaTransformers,
         CancellationToken cancellationToken)
     {
-        var description = ReasonPhrases.GetReasonPhrase(statusCode);
         var response = new OpenApiResponse
         {
-            Description = description,
+            Description = apiResponseType.Description ?? ReasonPhrases.GetReasonPhrase(statusCode),
             Content = new Dictionary<string, OpenApiMediaType>()
         };
 
diff --git a/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs b/src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Responses.cs
@@ -304,4 +304,69 @@ await VerifyOpenApiDocument(builder, document =>
             });
         });
     }
+
+    [Fact]
+    public async Task GetOpenApiResponse_UsesDescriptionSetByUser()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        const string expectedCreatedDescription = "A new todo item was created";
+        const string expectedBadRequestDescription = "Validation failed for the request";
+
+        // Act
+        builder.MapGet("/api/todos",
+            [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created, Description = expectedCreatedDescription)]
+        [ProducesResponseType(StatusCodes.Status400BadRequest, Description = expectedBadRequestDescription)]
+        () =>
+            { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = Assert.Single(document.Paths["/api/todos"].Operations.Values);
+            Assert.Collection(operation.Responses.OrderBy(r => r.Key),
+                response =>
+                {
+                    Assert.Equal("201", response.Key);
+                    Assert.Equal(expectedCreatedDescription, response.Value.Description);
+                },
+                response =>
+                {
+                    Assert.Equal("400", response.Key);
+                    Assert.Equal(expectedBadRequestDescription, response.Value.Description);
+                });
+        });
+    }
+
+    [Fact]
+    public async Task GetOpenApiResponse_UsesStatusCodeReasonPhraseWhenExplicitDescriptionIsMissing()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapGet("/api/todos",
+            [ProducesResponseType(typeof(TimeSpan), StatusCodes.Status201Created, Description = null)] // Explicitly set to NULL
+        [ProducesResponseType(StatusCodes.Status400BadRequest)] // Omitted, meaning it should be NULL
+        () =>
+            { });
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            var operation = Assert.Single(document.Paths["/api/todos"].Operations.Values);
+            Assert.Collection(operation.Responses.OrderBy(r => r.Key),
+                response =>
+                {
+                    Assert.Equal("201", response.Key);
+                    Assert.Equal("Created", response.Value.Description);
+                },
+                response =>
+                {
+                    Assert.Equal("400", response.Key);
+                    Assert.Equal("Bad Request", response.Value.Description);
+                });
+        });
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -274,7 +274,7 @@ private static RequestDelegateFactoryContext CreateFactoryContext(RequestDelegat`
`274`	`274`	`var serviceProvider = options?.ServiceProvider ?? options?.EndpointBuilder?.ApplicationServices ?? EmptyServiceProvider.Instance;`
`275`	`275`	`var endpointBuilder = options?.EndpointBuilder ?? new RdfEndpointBuilder(serviceProvider);`
`276`	`276`	`var jsonSerializerOptions = serviceProvider.GetService<IOptions<JsonOptions>>()?.Value.SerializerOptions ?? JsonOptions.DefaultSerializerOptions;`
`277`		`- var formDataMapperOptions = new FormDataMapperOptions();;`
	`277`	`+ var formDataMapperOptions = new FormDataMapperOptions();`
`278`	`278`
`279`	`279`	`var factoryContext = new RequestDelegateFactoryContext`
`280`	`280`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1 +1,3 @@`
`1`	`1`	`#nullable enable`
	`2`	`+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.get -> string?`
	`3`	`+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiResponseType.Description.set -> void`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Licensed to the .NET Foundation under one or more agreements.`
	`1`	`+// Licensed to the .NET Foundation under one or more agreements.`
`2`	`2`	`// The .NET Foundation licenses this file to you under the MIT license.`
`3`	`3`
`4`	`4`	`using System;`
`@@ -60,6 +60,8 @@ public class CustomResponseTypeAttribute : Attribute, IApiResponseMetadataProvid`
`60`	`60`
`61`	`61`	`public int StatusCode { get; set; }`
`62`	`62`
	`63`	`+ public string Description { get; set; }`
	`64`	`+`
`63`	`65`	`public void SetContentTypes(MediaTypeCollection contentTypes)`
`64`	`66`	`{`
`65`	`67`	`}`