Setting route order imperatively

[Tratcher]

Background and Motivation

People frequently ask how to change a route's order. Today this is only easy with attribute routing, not minimal APIs.

The workaround is: app.Map("/api/v1/users", emptyDelegate).Add(static builder => ((RouteEndpointBuilder)builder).Order = 1);

This comes up in YARP, migrations, internal customers, and external: #39241.

Proposed API

namespace Microsoft.AspNetCore.Builder;

public static class RoutingEndpointConventionBuilderExtensions
{
+    public static TBuilder WithOrder<TBuilder>(this TBuilder builder, int order) where TBuilder : IEndpointConventionBuilder;
}

Usage Examples

app.Map("/api/v1/users", ignoredDelegate);
app.Map("/api/v1/users", emptyDelegate).WithOrder(-1);

Alternative Designs

Risks

Not all routes support order. This would throw when used on a builder that didn't cast to RouteEndpointBuilder.

@danroth27

[Kahbazi]

@Tratcher Could I send a PR for this when the API got approved?

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[halter73]

It's very evident from reading the code that they went out of their way not to expose Order outside of MVC. I wonder if @JamesNK @davidfowl or @rynowak have context on the reasoning.

Personally, I like making it easy to override endpoints if you want to using order. I've written the workaround code a number of times myself, but maybe it's a footgun.

[davidfowl]

I don't see an issue with this API.

[rynowak]

It's very evident from reading the code that they went out of their way not to expose Order outside of MVC. I wonder if @JamesNK @davidfowl or @rynowak have context on the reasoning.

Aside from being a footgun (it definitely is), order can have a surprising impact on performance for a really big route table. Those are some of the reasons why didn't make it obvious how to use order. You shouldn't need it that often.

No concerns here, if the question comes up often then it should be made easier.

[JamesNK]

What are scenarios where this would be used? In this issue - #39241 - there are two ambiguous routes, and setting the order means one will never be called. I wonder why someone would want to do that.

Could MapFallback be used instead? That sets order to the lowest possible value.

[halter73]

API Review Notes:

• Is lower or higher order negative priority? Negative.
  • This isn't intuitive, but it's consistent with the property.
• Will this apply to groups of endpoints?
  • Of course, it targets IEndpointConventionBuilder
  • Mvc has an attribute for this, could this be overridden by the group?
    • We don't think so, but we should definitely add tests to ensure that the more-local MVC attributes win over the group.
    • We should also verify that calling WithOrder on the ControllerActionEndpointConventionBuilder doesn't override attributes.
    • In the group case, we want the attribute to be preferred (so we will use IEndpointConventionBuilder.Add instead of IEndpointConventionBuilder.Finally.

       var group = app.MapGroup("/order").WithOrder(-1);
       group.MapControllers();

    • In the ControllerActionEndpointConventionBuilder, we want the WithOrder to be preferred. This is different from groups. We assume this would be the case for .Add(static builder => ((RouteEndpointBuilder)builder).Order = -1)

       app.MapControllers().WithOrder(-1);

• What do we do in WithOrder if EndpointBuilder is not a RouteEndpointBuilder?
  • Do we skip them or throw an InvalidOperationException?
  • Would it be problematic if some InertEndpointBuilders from MapDynamicControllerRoute or other types are in the mix?
  • How would you troubleshoot if this does nothing?
  • We are going to throw InvalidOperationException for now, and see how bad it is. It's less breaking to skip later.
• Do we want to define a new interface for IEndpointConventionBuilder implementations implement to support order so things like MapControllers can choose not to opt-in?
  • Don't we want to be able to use this MapController?
• Does this API give you enough control on override behavior in groups? What if I wanted the order to be changed in a IEndpointConventionBuilder.Finally callback?
  • Do we add a new bool parameter called overrideOrder or something?
• What if we extended EndpointBuilder instead of IEndpointConventionBuilder, so you still wouldn't have to cast, but you would control weather you call Add or `Finally?
  • Ex: app.Map("/api/v1/users", emptyDelegate).Add(static builder => builder.WithOrder(-1))
  • This really hurts discoverability.
• Does RoutingEndpointConventionBuilderExtensions make sense?
  • It includes WithHost, WithName and other similar methods. Seems reasonable.

API Approved. Let's make sure we throw for non-RouteEndpointBuilder early on.

namespace Microsoft.AspNetCore.Builder;

public static class RoutingEndpointConventionBuilderExtensions
{
+    public static TBuilder WithOrder<TBuilder>(this TBuilder builder, int order) where TBuilder : IEndpointConventionBuilder;
}

[Tratcher]

@Kahbazi This is ready, want to send a PR now?

[Kahbazi]

@Kahbazi This is ready, want to send a PR now?

@Tratcher I will send a PR next week if it's not too late.

[Kahbazi]

@Kahbazi This is ready, want to send a PR now?

@Tratcher I will send a PR next week if it's not too late.

@Tratcher I finally send a PR for this. What a long weeeek! 😅