Setting the Description in a ProducesResponseTypeAttribute works correctly for Minimal API

[sander1095]

Setting the Description in a ProducesResponseTypeAttribute works correctly for Minimal API

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Using ProducesResponseType with the Description property works with Minimal API

Description

This PR fixes the issue where the description is not set in a Minimal API application:

app.MapGet("/weatherforecast",
[ProducesResponseType<IEnumerable<WeatherForecast>>(StatusCodes.Status200OK, Description = "The service is healthy.")]
() =>
{
    return new Something();
}

This did work for Controllers AND for Minimal API's when the method body was empty. The tests sadly did not cover scenarios where the method body did have content.

This PR adds test cases in several places to check that the Description is added correctly to the OpenAPI document. It also fixes the underlying issues.

Please perform a careful review of the tests in the ApiExlorer project! I made some changes and am not 100% sure that the tests are 100% valid and represent real scenarios.

Fixes #60518

[dotnet-policy-service]

Thanks for your PR, @sander1095. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[captainsafia]

Is there a reason the existing logic in the ApiResponseTypeProvider doesn't handle this? That feels like the right place to do this instead of having to filter on the types returned by it to set the description.

[sander1095]

Hey @captainsafia ! Great question! We need to discuss how we want to approach this.

Short answer: The ApiRespnseTypeProvider does set the description, but the EndpointMetadataProvider throws it away. I found out about this specific behavior thanks to your comment. Below is the explanation and some suggestions on ways we could perhaps make the code a bit more clear:

I dove a bit deeper into this, and this is what happens:

The code above your comment does this (AddSupportedResponseTypes() in EndpointMetadataApiDescriptionProvider.cs)

var responseProviderMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(
    responseProviderMetadata, responseType, defaultErrorType, contentTypes, out var errorSetByDefault);
var producesResponseMetadataTypes = ApiResponseTypeProvider.ReadResponseMetadata(producesResponseMetadata, responseType);

• responseProviderMetadataTypes contains the Description set in ApiResponseTypeProvider, as you correctly said!
• producesResponseMetadataTypes does not contain the Description.

Next, the following happens:

// We favor types added via the extension methods (which implements IProducesResponseTypeMetadata)
// over those that are added via attributes.
var responseMetadataTypes = producesResponseMetadataTypes.Values.Concat(responseProviderMetadataTypes.Values);
if (responseMetadataTypes.Any())
{
    foreach (var apiResponseType in responseMetadataTypes)
    {

        // Code you commented on happens here
   
        if (!supportedResponseTypes.Any(existingResponseType => existingResponseType.StatusCode == apiResponseType.StatusCode))
        {
            supportedResponseTypes.Add(apiResponseType);
        }
    }
}

SupportedResponseTypes is eventually returned and used.

As you can see, both lists are combined with producesResponseMetadataTypes being iterated over first, and thus that's the first apiResponseType that's added to the supportedResponseTypes, causing it to not contain a description.

At some point we loop over the responseProviderMetadataTypes which DO contain the description, but that doesn't get added to the list of supportedResponseTypes because that type already exists thanks to the metadata.

---

My code forces the Description to be set when we loop over the first iteration of the metadata, which fixes the issue. But this is a bit confusing, now that I understand the code a bit better.

What would you like as a solution? The core problem is that metadata is iterated over first, which doesn't contain the description. It then fails to enrich the response types from ApiDescriptionProvider which supplies the description.

• IProducesResponseTypeMetadata should also have a Description.
  • On line 330 we call endpointMetadata.GetOrderedMetadata<IProducesResponseTypeMetadata>();. This has a Description property, but is null. I'm not sure if this is intended, or if I forgot to set the Description for metadata in a previous PR. (Please advise if this is desirable/possible).
  • If it is desired that IProducesResponseTypeMetadata has a Description, I will need to make that happen AND update line 247 in ApiDescriptionProvider to set the description.
• Setting description once
  • I can keep the code the same, so it uses the first non-null description
  • I could change my code to use the LAST description it encounters in the list, in case multiple descriptions are set
• Keep code the same, add comments to explain it
• Move the code that sets the description out of the loop and populate the Descriptions after the loop, based on the data from responseProviderMetadataTypes. This makes the code more explicit.
• Other thoughts are welcome!

---

This also explains why this bug happens in Minimal API's and not in Controllers. I believe EndpointMetadataApiDescriptionProvider is only used for Minimal API, whereas ApiDescriptionProvider is used for both Controllers and Minimal API. Because the EndpointMetadataApiDescriptionProvider forgets to set enrich response types it has already added to the list, the description goes missing.

[captainsafia]

@sander1095 Thanks for digging into this and sharing detailed notes. Based on this analysis, I expect this option:

I could change my code to use the LAST description it encounters in the list, in case multiple descriptions are set

To be the prefered one since it aligns most closely with the way metadata gets respected in other scenarios. We'll have to sanity check this implementation with multiple ProducesResponseType attributes on the same endpoint to make sure the additions only apply to the target type and status code.

[sander1095]

@captainsafia Done! See the latest commits