diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index c920ea01d8aa..89d45efc5e11 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -405,7 +405,7 @@ private static void AddSupportedResponseTypes( foreach (var metadata in responseMetadataTypes) { if (metadata.StatusCode == apiResponseType.StatusCode && - metadata.Type == apiResponseType.Type && + TypesAreCompatible(apiResponseType.Type, metadata.Type) && metadata.Description is not null) { matchingDescription = metadata.Description; @@ -413,6 +413,20 @@ private static void AddSupportedResponseTypes( } return matchingDescription; } + + static bool TypesAreCompatible(Type? apiResponseType, Type? metadataType) + { + // We need to a special check for cases where the inferred type is different than the one specified in attributes. + // For example, an endpoint that defines [ProducesResponseType>], + // but the endpoint returns weatherForecasts.ToList(). Because List<> is a different type than IEnumerable<>, it would incorrectly set OpenAPI metadata incorrectly. + // We use a conservative unidirectional check where the attribute type must be assignable from the inferred type. + // This handles inheritance (BaseClass ← DerivedClass) and interface implementation (IEnumerable ← List). + // This should be sufficient, as it's more common to specify an interface or base class type in the attribute and a concrete type in the endpoint implementation, + // compared to doing the opposite. + // For more information, check the related bug: https://github.com/dotnet/aspnetcore/issues/60518 + return apiResponseType == metadataType || + metadataType?.IsAssignableFrom(apiResponseType) == true; + } } private static ApiResponseType CreateDefaultApiResponseType(Type responseType) diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index 9a5233a709a1..7de4d5eb5b89 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -331,6 +331,108 @@ public void AddsResponseDescription() Assert.Equal(expectedBadRequestDescription, badRequestResponseType.Description); } + [Fact] + public void AddsResponseDescription_WorksWithGenerics() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => new GenericClass { Value = new TimeSpan() }); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(GenericClass), okResponseType.Type); + Assert.Equal(typeof(GenericClass), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithGenericsAndTypedResults() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => TypedResults.Ok(new GenericClass { Value = new TimeSpan() })); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(GenericClass), okResponseType.Type); + Assert.Equal(typeof(GenericClass), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollections() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => new List { new() }); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(List), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(List), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + [Fact] + public void AddsResponseDescription_WorksWithCollectionsAndTypedResults() + { + const string expectedOkDescription = "The weather forecast for the next 5 days."; + + var apiDescription = GetApiDescription([ProducesResponseType>(StatusCodes.Status200OK, Description = expectedOkDescription)] + () => TypedResults.Ok(new List { new() })); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(List), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes) + Assert.Equal(typeof(List), okResponseType.ModelMetadata?.ModelType); + Assert.Equal(expectedOkDescription, okResponseType.Description); + + var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats); + Assert.Equal("application/json", createdOkFormat.MediaType); + } + + /// + /// EndpointMetadataApiDescriptionProvider performs a one way type check for discovering response types to match the description that's set in [ProducesResponseType] + /// The reason we do a one-way check instead of a bidirectional check is to prevent too many positive matches. + /// + /// + /// Example: If we did a bidirectional check, we would match something scenarios like this, which can cause confusion: + /// [ProducesResponseType(StatusCodes.Status200OK, Description = "Returned with a string")] -> TypedResults.Ok(new object()) + /// This would match because object is assignable to string, + /// but it doesn't make sense to add the Description to the object type because the attribute says we should return a string. + /// + /// This test documents this desired behavior and will fail if the behavior changes, so the developer can double check if their change is intentional. + /// + [Fact] + public void AddsResponseDescription_ShouldFailWhenInferredTypeIsNotDirectlyAssignableToAttributeType() + { + var apiDescription = GetApiDescription([ProducesResponseType(StatusCodes.Status200OK, Description = "Only returned with a string")] + () => TypedResults.Ok(new object())); + + var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes); + + Assert.Equal(200, okResponseType.StatusCode); + Assert.Equal(typeof(object), okResponseType.Type); + Assert.Equal(typeof(object), okResponseType.ModelMetadata?.ModelType); + Assert.Null(okResponseType.Description); + } + [Fact] public void WithEmptyMethodBody_AddsResponseDescription() { @@ -1814,4 +1916,6 @@ private class TestServiceProvider : IServiceProvider return null; } } + + private class GenericClass { public required TType Value { get; set; } } }