aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Different behavior for guid constraint with Swagger in Controllers and Minimal APIs

Open marcominerva opened this issue 3 years ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

There is a different behavior in the swagger.json file that is generated in a Controller-based project and in a project that uses Minimal APIs, when there is a guid constraint. In Controller-based projects, the swagger.json file specifies the uuid format in the schema section for the guid parameter. However, no format attribute is written in the schema section when working with Minimal APIs.

Expected Behavior

The swagger.json file should be the same for both project types.

Steps To Reproduce

I'm using the default Swagger configuration that comes with the .NET 6.0 templates. Using a Controller like this:

[ApiController]
[Route("[controller]")]
public class PeopleController : ControllerBase
{
    [HttpGet("{id:guid}")]
    public IActionResult Get(Guid id)
        => Ok(new { FirstName = "Donald", LastName = "Duck" });
}

The generated swagger.json files specifies the uuid format for the id parameter:

"/People/{id}": {
  ...
  "parameters": [
    {
      "name": "id",
      "in": "path",
      "required": true,
      "schema": {
        "type": "string",
        "format": "uuid"
      }
    }
  ],

If, however, I create a Minimal API:

app.MapGet("/People/{id:guid}", (Guid id) =>
{
    return Results.Ok(new { FirstName = "Donald", LastName = "Duck" });
});

Then the swagger.json contains the following description:

"/People/{id}": {
  ...
  "parameters": [
    {
      "name": "id",
      "in": "path",
      "required": true,
      "schema": {
        "type": "string"
      }
    }
  ],

As you can see, no format attribute is written in the schema section.

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

ASP.NET Core version: 6.0 IDE: Visual Studio 2022 17.0.5

marcominerva avatar Jan 31 '22 09:01 marcominerva

There's a couple of things to verify as we investigate this:

  • Is the Type correctly processed from ParameterInfo for the id parameter in both scenarios above? From a brief glance, both EndpointMetadataApiDescriptionProvider and DefaultDescriptionProvider seem to be doing the same thing here.
  • Does Swashbuckle process the guid constraint on the route template for the minimal API and MVC in the same way? https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/1096 might be a related issue to look into here.
  • Is this causing the problem?

https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/844e11cb202c96d3fb4e4ef551aba4af3ba9e6e0/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/OpenApiSchemaExtensions.cs#L67-L68

captainsafia avatar Feb 01 '22 21:02 captainsafia

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost avatar Feb 01 '22 22:02 ghost

  • Is the Type correctly processed from ParameterInfo for the id parameter in both scenarios above? From a brief glance, both EndpointMetadataApiDescriptionProvider and DefaultDescriptionProvider seem to be doing the same thing here. https://github.com/domaindrivendev/Swashbuckle.AspNetCore/blob/844e11cb202c96d3fb4e4ef551aba4af3ba9e6e0/src/Swashbuckle.AspNetCore.SwaggerGen/SchemaGenerator/OpenApiSchemaExtensions.cs#L67-L68

The type is correctly processed as string in both cases. The problem is related to the missing format attribute in the schema section for Minimal APIs (while with Controllers it is correctly set to uuid).

Yes, the issue is related to this one, but it happens only with Minimal APIs, Controllers works correctly.

marcominerva avatar Feb 02 '22 08:02 marcominerva

Took the time to investigate this. It looks like this bug is happening because of the parameter binding logic that is implemented in minimal APIs. Parameter types are try-parseable intro strings are automatically mapped as string types.

https://github.com/dotnet/aspnetcore/blob/dcbfb829777da26078b31bace83736e6f3bf3295/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs#L291-L292

Similar behavior happens for things like DateTime, TimeSpan, etc.

captainsafia avatar Aug 19 '22 20:08 captainsafia

@marcominerva Meanwhile it's get delivered in .NET 7 (Hopefully), you can add a filter to your swagger configuration to set the guid parameter as 'uuid'.

Guid Parameter Filter:

 public class GuidParameterFilter : IParameterFilter
 {
     public void Apply(OpenApiParameter parameter, ParameterFilterContext context)
     {
         if (parameter.Schema.Type == "string" &&
             context.ApiParameterDescription.Type == typeof(Guid))
         {
             parameter.Schema.Format = "uuid";
         }
     }
 }

Program.cs:

services.AddSwaggerGen(c =>
{
   c.ParameterFilter<GuidParameterFilter>();
}

Solution

For the other parameter bindings there as @captainsafia mentioned, it should be applied for any other type like DateTime, TimeSpan, etc; even may need to have other considerations and complexity. If you find a better solution please share.

fcortes18 avatar Oct 11 '22 05:10 fcortes18

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost avatar Oct 11 '22 19:10 ghost

Hi all,

We just posted an announcement about the future of OpenAPI support in ASP.NET Core. TL;DR: we're adding OpenAPI document generation as a first-class feature in ASP.NET Core.

I've included the GUID scenario mentioned here into our test bed:

    // Coverage for https://github.com/dotnet/aspnetcore/issues/39886
    [Fact]
    public async Task GenerateOpenApIDocument_WithGuidParameter_Controller()
        => await VerifyOpenApiDocument("/test-with-guid-param", nameof(TestController.TestWithGuidParameter));

    [Fact]
    public async Task GenerateOpenApIDocument_WithGuidParameter()
        => await VerifyOpenApiDocument("/test-with-guid-param", (Guid id) => { });

And have validated that for both cases we generate the correct schema:

"parameters": [
{
  "name": "id",
  "in": "query",
  "required": true,
  "schema": {
    "type": "string",
    "format": "uuid"
  }
}
],

captainsafia avatar Mar 18 '24 18:03 captainsafia

Closing as this is resolved with our new built-in OpenAPI support.

captainsafia avatar Apr 23 '24 20:04 captainsafia