Skip to content

Endpoint name metadata should be set automatically to the method name when Map overload is called passing a MethodGroup #34540

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

Closed
Tracked by #34514
DamianEdwards opened this issue Jul 20, 2021 · 6 comments
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:1 Work that is critical for the release, but we could probably ship without
Milestone

Comments

@DamianEdwards
Copy link
Member

DamianEdwards commented Jul 20, 2021

Endpoint names are used to lookup endpoints when generating links using LinkGenerator, and as their unique per application are a good candidate for using as the operationId for an endpoint in OpenAPI (Swagger) documents.

(domaindrivendev/Swashbuckle.AspNetCore#2165 is tracking setting the operationId from endpoint name by default in Swashbuckle.)

Endpoint names are set using the Microsoft.AspNetCore.Routing.IEndpointNameMetadata interface. In the framework today this can be set imperatively by adding an instance of Microsoft.AspNetCore.Routing.EndpointNameMetadata to the endpoint's metadata, e.g. builder.WithMetadata(new EndpointNameMetadata("GetTodoById")). Issues #34538 and #34539 are tracking providing further ways to define endpoint names manually.

In minimal APIs we should automatically set the endpoint name to the value of MethodInfo.Name when the endpoint is defined using a MethodGroup rather than a lambda or anonymous delegate, e.g.:

// These endpoints do not have names
app.MapGet("/hello", () => "Hello");
app.MapGet("/goodbye/{name}", (string name) => $"Goodbye {name}");

// These endpoints have their name set automatically
app.MapGet("/todos/{id}", GetTodoById);
app.MapPost("/todos", AddTodo);

async Task<IResult> GetTodoById(int id, TodoDb db)
{
    return await db.Todos.FindAsync(id)
        is Todo todo
            ? Results.Ok(todo)
            : Results.NotFound();
};

async Task<IResult> AddTodo(Todo todo, TodoDb db)
{
    db.Todos.Add(todo);
    await db.SaveChangesAsync();

    return Results.CreatedAtRoute(nameof(GetTodoById), todo);
}
@DamianEdwards DamianEdwards added area-runtime feature-minimal-actions Controller-like actions for endpoint routing labels Jul 20, 2021
@ghost
Copy link

ghost commented Jul 20, 2021

Thanks for contacting us.

We're moving this issue to the Next sprint 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.

@rafikiassumani-msft rafikiassumani-msft added the Priority:1 Work that is critical for the release, but we could probably ship without label Jul 22, 2021
@davidfowl davidfowl added old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-runtime labels Jul 31, 2021
@DamianEdwards
Copy link
Member Author

@rafikiassumanimsft was this done? The PR I knew about is still in draft #35069

@rafikiassumani-msft
Copy link
Contributor

rafikiassumani-msft commented Aug 14, 2021

@DamianEdwards I learned today that we might not be able to do this at the moment due to the behavior implications in MVC. @javiercn @pranavkm @davidfowl . I am happy to re-open the issue and discuss further next week.

@captainsafia
Copy link
Member

Yep -- I think that would be helpful. I was OOF last week so would be good to get context on the reasoning behind this. I know @javiercn had posted a comment in the PR around have duplicate EndpointNames but wasn't sure it that was the only thing.

@davidfowl
Copy link
Member

I think the fact that local function names are mangled makes this less attractive. If we can make that work somehow then we can revisit...

@davidfowl davidfowl reopened this Aug 15, 2021
@DamianEdwards
Copy link
Member Author

Here's what I did in my playground app to enable this: https://github.com/DamianEdwards/MinimalApiPlayground/blob/main/src/MinimalApiPlayground/Properties/OpenApi.cs#L137

Fair warning, it assumes the format of compiler-generated method names and uses Regex to find the local function name. Ideally the compiler would add an attribute to the method to preserve the original name in the case the user provided one.

@ghost ghost locked as resolved and limited conversation to collaborators Sep 18, 2021
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-actions Controller-like actions for endpoint routing old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:1 Work that is critical for the release, but we could probably ship without
Projects
None yet
Development

No branches or pull requests

6 participants