Skip to content

Fix ProducesResponseType's Description not being set for Minimal API's when attribute and inferred types aren't an exact match #62695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -405,14 +405,28 @@ 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;
}
}
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<IEnumerable<WeatherForecast>>],
// 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<T> ← List<T>).
// 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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<GenericClass<TimeSpan>>(StatusCodes.Status200OK, Description = expectedOkDescription)]
() => new GenericClass<TimeSpan> { Value = new TimeSpan() });

var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes);

Assert.Equal(200, okResponseType.StatusCode);
Assert.Equal(typeof(GenericClass<TimeSpan>), okResponseType.Type);
Assert.Equal(typeof(GenericClass<TimeSpan>), 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<GenericClass<TimeSpan>>(StatusCodes.Status200OK, Description = expectedOkDescription)]
() => TypedResults.Ok(new GenericClass<TimeSpan> { Value = new TimeSpan() }));

var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes);

Assert.Equal(200, okResponseType.StatusCode);
Assert.Equal(typeof(GenericClass<TimeSpan>), okResponseType.Type);
Assert.Equal(typeof(GenericClass<TimeSpan>), 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<IEnumerable<TimeSpan>>(StatusCodes.Status200OK, Description = expectedOkDescription)]
() => new List<TimeSpan> { new() });

var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes);

Assert.Equal(200, okResponseType.StatusCode);
Assert.Equal(typeof(List<TimeSpan>), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes)
Assert.Equal(typeof(List<TimeSpan>), 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<IEnumerable<TimeSpan>>(StatusCodes.Status200OK, Description = expectedOkDescription)]
() => TypedResults.Ok(new List<TimeSpan> { new() }));

var okResponseType = Assert.Single(apiDescription.SupportedResponseTypes);

Assert.Equal(200, okResponseType.StatusCode);
Assert.Equal(typeof(List<TimeSpan>), okResponseType.Type); // We use List as the inferred type has higher priority than those set by metadata (attributes)
Assert.Equal(typeof(List<TimeSpan>), okResponseType.ModelMetadata?.ModelType);
Assert.Equal(expectedOkDescription, okResponseType.Description);

var createdOkFormat = Assert.Single(okResponseType.ApiResponseFormats);
Assert.Equal("application/json", createdOkFormat.MediaType);
}

/// <summary>
/// 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.
/// </summary>
/// <remarks>
/// Example: If we did a bidirectional check, we would match something scenarios like this, which can cause confusion:
/// [ProducesResponseType<string>(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.
/// </summary>
[Fact]
public void AddsResponseDescription_ShouldFailWhenInferredTypeIsNotDirectlyAssignableToAttributeType()
{
var apiDescription = GetApiDescription([ProducesResponseType<string>(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()
{
Expand Down Expand Up @@ -1814,4 +1916,6 @@ private class TestServiceProvider : IServiceProvider
return null;
}
}

private class GenericClass<TType> { public required TType Value { get; set; } }
}
Loading