[Minimal API] Route parameters are not set as required when using WithOpenApi and Nullable Reference Types are disabled

[marcominerva]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

If I define this endpoint:

#nullable enable
app.MapGet("/api/{group}/people_nullable_enabled", (string group) =>
{
    return TypedResults.Ok(new { GroupName = group });
})
.WithOpenApi();

swagger.json file correctly sets the group route parameter as required:

"parameters": [
  {
    "name": "group",
    "in": "path",
    "required": true,
    "style": "simple",
    "schema": {
      "type": "string"
    }
  }
]

However, If I disable Nullable Reference Types, than this parameter is no longer marked as required:

#nullable disable
app.MapGet("/api/{group}/people_nullable_disabled", (string group) =>
{
    return TypedResults.Ok(new { GroupName = group });
})
.WithOpenApi();

"parameters": [
  {
    "name": "group",
    "in": "path",
    "style": "simple",
    "schema": {
      "type": "string"
    }
  }
]

If I try to invoke the endpoint using Swagger UI, a comma is passed as route argument:

The problem is related to the WithOpenApi extension method. In fact, if I remove it, than the group route parameter is again set as required:

#nullable disable
app.MapGet("/api/{group}/people_nullable_disabled", (string group) =>
{
    return TypedResults.Ok(new { GroupName = group });
});

"parameters": [
  {
    "name": "group",
    "in": "path",
    "required": true,
    "style": "simple",
    "schema": {
      "type": "string"
    }
  }
]

Expected Behavior

The group route parameter should be required in any cases, regardless the presence of Nullable Reference Types and the usage of WithOpenApi.

Steps To Reproduce

Minimal repro here: https://github.com/marcominerva/PathIssue

Exceptions (if any)

No response

.NET Version

7.0.103

Anything else?

No response

[nanny07]

I think the problem could be this statement:
https://github.com/dotnet/dotnet/blob/8d5c52c5dd54417a2071c196663bb29ab1ee2442/src/aspnetcore/src/OpenApi/src/OpenApiGenerator.cs#L381

[captainsafia]

This is expected behavior based on the way optionality rules work in minimal APIs (in both the RequestDelegateFactory and the OpenAPI generator). If nullability is disabled in an application context, then we always assume that a parameter is optional (because it's not possible to guarantee that it is required). If nullability is enabled, we treat a parameter as optional if it has a default value or if it has been annotated with the nullability modifier.

Your observation that this behavior doesn't make sense for route parameters that are part of the routing path is a sensible one, and highlights the distinction in behavior between what happens at the routing layer (the group is required) and what happens in the handler (the group is optional).

I think the problem could be this statement:
https://github.com/dotnet/dotnet/blob/8d5c52c5dd54417a2071c196663bb29ab1ee2442/src/aspnetcore/src/OpenApi/src/OpenApiGenerator.cs#L381

The behavior here is correct because it matches the logic that the RequestDelegateFactory uses to construct the parameter binding logic, see https://github.com/dotnet/dotnet/blob/8d5c52c5dd54417a2071c196663bb29ab1ee2442/src/aspnetcore/src/Http/Http.Extensions/src/RequestDelegateFactory.cs#L2020-L2036.

Marking as investigate to see if it is possible to make the generator smarter about parameters needed in the route but not the handler.

[ghost]

To learn more about what this message means, what to expect next, and how this issue will be handled you can read our Triage Process document.
We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. Because it's not immediately obvious what is causing this behavior, we would like to keep this around to collect more feedback, which can later help us determine how to handle this. 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 work.

[marcominerva]

This is expected behavior based on the way optionality rules work in minimal APIs (in both the RequestDelegateFactory and the OpenAPI generator). If nullability is disabled in an application context, then we always assume that a parameter is optional (because it's not possible to guarantee that it is required). If nullability is enabled, we treat a parameter as optional if it has a default value or if it has been annotated with the nullability modifier.

Your observation that this behavior doesn't make sense for route parameters that are part of the routing path is a sensible one, and highlights the distinction in behavior between what happens at the routing layer (the group is required) and what happens in the handler (the group is optional).

So I think this behavior should be aligned in the Swagger UI as well, otherwise we have the issue I mentioned before, do you agree?

[captainsafia]

@marcominerva Yes, you're correct. I'd like to retract the explainer that I provided in #46746 (comment).

The behavior I mentioned is accurate for non-route parameters. OpenAPI requires that all route parameters be marked as required, which is incongruent with the concept of optional route parameters that we have in ASP.NET Core. I believe the correct thing to do here is to treat all route parameters as required.

I'll fix this in the OpenApiGenerator and as part of the new built-in OpenAPI document generation support.

[captainsafia]

The built-in OpenAPI support now follows the correct behavior per the OpenAPI spec and treats all parameters coming from the path as required regardless of their nullability status.

I've added a test case in #55321 to cover this.

In the future, I plan on replating the WithOpenApi implementation to work as an operation transformer under the hood and getting rid of the OpenAPI generator altogether. This should unify the way OpenAPI operations are represented across the stack.

Closing this is as resolved for now.