Skip to content
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

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

Closed
1 task done
marcominerva opened this issue Jan 31, 2022 · 8 comments
Closed
1 task done
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing feature-openapi
Milestone

Comments

@marcominerva
Copy link
Contributor

marcominerva commented Jan 31, 2022

Is there an existing issue for this?

  • 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

@javiercn javiercn added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jan 31, 2022
@davidfowl davidfowl added the feature-minimal-actions Controller-like actions for endpoint routing label Jan 31, 2022
@captainsafia
Copy link
Member

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? Swagger ignores route parameter constraint domaindrivendev/Swashbuckle.AspNetCore#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

@rafikiassumani-msft rafikiassumani-msft added this to the .NET 7 Planning milestone Feb 1, 2022
@ghost
Copy link

ghost commented Feb 1, 2022

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.

@marcominerva
Copy link
Contributor Author

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.

@captainsafia
Copy link
Member

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.

var displayType = !parameter.ParameterType.IsPrimitive && Nullable.GetUnderlyingType(parameter.ParameterType)?.IsPrimitive != true
? typeof(string) : parameter.ParameterType;

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

@fcortes18
Copy link

fcortes18 commented Oct 11, 2022

@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.

@ghost
Copy link

ghost commented Oct 11, 2022

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.

@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
@captainsafia captainsafia removed the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jun 6, 2023
@captainsafia
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing feature-openapi
Projects
No open projects
Status: No status
Development

No branches or pull requests

7 participants