Skip to content

Routing order with path match and a MapDynamicControllerRoute #21083

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
sitepodmatt opened this issue Apr 22, 2020 · 5 comments
Closed

Routing order with path match and a MapDynamicControllerRoute #21083

sitepodmatt opened this issue Apr 22, 2020 · 5 comments
Assignees
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing
Milestone

Comments

@sitepodmatt
Copy link

I beleive this could possibly be related to #16579. We're using ASP.Net Core 3.1 LTS with MVC Controllers and have set of static routes plus a whole lot of dynamic routes for categories, products and other slug styles (driven by an inmemory cache of some database view).. milliseconds are important on our GET pipeline so we noticed something odd but not breaking as the correct route is eventually resolved.

If I wire up a simple scenario of match homepage, and a DynamicRouteValueTransformer that has a big cache from a database to decide which controller/action to select, then I expect only the match for the homepage to be hit when I GET ?, but instead it goes through TransformAsync on RouteResolver too (with no routevalues set) - whilst it works it seems inefficient and adds several milliseconds to our request processing.

  app.UseEndpoints(endpoints =>
            {
                 endpoints.MapGet("/", async context =>
                 {
                     await context.Response.WriteAsync("Homepage");
                 });
                 //HomeController.MapRoutes(endpoints);
                 endpoints.MapDynamicControllerRoute<RouteResolver>("/{**path}");
                // ContentController.MapRoutes(endpoints);
            });

Scenario: GET /
Expected: TransformAsync on RouteResolve not to be called.
Actual: TransformAsync on RouteResolve is needlessly called.

@sitepodmatt
Copy link
Author

sitepodmatt commented Apr 22, 2020

@rynowak .

@javiercn javiercn added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Apr 22, 2020
@mkArtakMSFT
Copy link
Contributor

Thanks for contacting us.
@javiercn can you please look into this? Thanks!

@rynowak
Copy link
Member

rynowak commented Apr 22, 2020

I expect only the match for the homepage to be hit when I GET

This seems like a reasonable expectation from the outside, but it's not how routing really works. Things like DynamicControllerRoute have the ability to expand and change the set of endpoints. So while there's a logical processing order of endpoints, routing system in actuality processes them all at once.

It is done this way because:

  • It's more efficient for loads of common cases
  • Gives us a predictable performance profile by removing backtracking from the algorithm
  • Improves a bunch of extensibility scenarios

The case that we're optimizing for here is where there's a complex policy that comes after your DynamicControllerRoute has a chance to run. The sequence of step basically looks like:

path matching -> constraints -> dynamic policies (you are here) -> other policies

Other policies includes things like:

  • http methods
  • content types ([Consumes(...)])
  • hostname
  • versioning

Since dynamic policies can change the set of endpoints being considered, they need to run before those things, so that they can see the endpoints returned from your policy as well as others and consider them together.


We could consider adding features to DynamicControllerRoute for this scenario, but I'm not sure if they match what you really will want, so let's discuss.

Since you're interested in avoiding your logic running - what kind of criteria would you base that on?

  • Did any other endpoints match? (this seems like something we could code)
  • Do you want to negate another path? (match /{**path} but not /)
  • other ideas?

@sitepodmatt
Copy link
Author

sitepodmatt commented Apr 23, 2020

Rynowak, thanks for the very detailed explanation, this makes more sense now. Yes I'd like to avoid running logic if there is a match already. I think I'm going to approach this different with this new understanding. UseRouting() to match stuff like /, /about /contact us, and then another middleware after routing that checks if GetEndpoint() is null and if so does the custom matching and then .SetEndpoint - just need to figure out how to get/create the relevant Endpoint - any pointers? thanks

@ghost
Copy link

ghost commented Nov 12, 2020

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

@ghost ghost closed this as completed Nov 12, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing
Projects
None yet
Development

No branches or pull requests

4 participants