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

OpenAPI documentation is incorrect when using custom TryParse() binding with Minimal APIs #58318

Closed
1 task done
marcominerva opened this issue Oct 9, 2024 · 3 comments · Fixed by #58350
Closed
1 task done
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-openapi

Comments

@marcominerva
Copy link
Contributor

marcominerva commented Oct 9, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have used the same title of #36412 because the same issue happens today using the new OpenAPI documentation generator that is built-in in .NET 9.0.

With this code:

app.MapGet("/student/{student}", (Student student) => $"Hi {student.Name}");

public record Student(string Name)
{
    public static bool TryParse(string value, out Student? result)
    {
        if (value is null)
        {
            result = null;
            return false;
        }

        result = new Student(value);
        return true;
    }
}

OpenAPI documentation is wrongly created as the following:

"parameters": [
    {
        "name": "student",
        "in": "query",
        "required": true,
        "schema": {
            "$ref": "#/components/schemas/Student"
        }
    }
],
// ...
"components": {
    "schemas": {
        "Student": {
            "required": [
                "name"
            ],
            "type": "object",
            "properties": {
                "name": {
                    "type": "string"
                }
            }
        }
    }
},

Expected Behavior

Parameters that use a TryParse custom binding should be configured as string in the openapi/v1.json file:

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

Steps To Reproduce

Minimal repro: https://github.com/marcominerva/TryParseOpenApiIssue

Exceptions (if any)

No response

.NET Version

9.0.0-rc.2.24474.3

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Oct 9, 2024
@martincostello martincostello added feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Oct 9, 2024
@captainsafia
Copy link
Member

@marcominerva Thanks for filing this issue!

I believe that this was a regression that was introduced in the model binding layer in this commit although I'll have to debug a little further. I'm a little surprised that our AddsFromRouteParameterAsPathWithCustomClassWithTryParse test didn't catch this although the actual issue might be something else.

It seems like you also reproed this with .NET 9 and Swashbuckle? If so, that would point more towards it being an ApiExplorer problem as opposed to a something in the M.A.OpenApi package. I'll try to take a further look at this.

@captainsafia captainsafia self-assigned this Oct 9, 2024
@marcominerva
Copy link
Contributor Author

marcominerva commented Oct 9, 2024

Yes @captainsafia, it seems that the issue happens also with the latest version of Swashbuckle.AspNetCore: domaindrivendev/Swashbuckle.AspNetCore#3105.

Thank you for your support!

@marcominerva
Copy link
Contributor Author

@captainsafia I noticed that the issue is solved only if I don't use .Accepts<T>:

// Will correcly report a student as a query string parameter
app.MapPost("/enroll", (Student student) => Results.NoContent());

// Will report a student as a query string parameter and as the request body
app.MapPost("/enroll-withaccepts", (Student student) => Results.NoContent())
   .Accepts<Student>("application/json");

// Will correctly report the parameter as a string
app.MapGet("/student/{student}", (Student student) => $"Hi {student.Name}");

// Will correctly report the parameter as a string and a the request body
app.MapGet("/student-withaccepts/{student}", (Student student) => $"Hi {student.Name}")
    .Accepts<Student>("application/json");

app.Run();

public record Student(string Name)
{
    public static bool TryParse(string value, out Student? result)
    {
        if (value is null)
        {
            result = null;
            return false;
        }

        result = new Student(value);
        return true;
    }
}

For example:
Image

I have updated the repo at https://github.com/marcominerva/TryParseOpenApiIssue with this new scenarios.

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-openapi
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants