aspnet / Mvc

[Archived] ASP.NET Core MVC is a model view controller framework for building dynamic web sites with clean separation of concerns, including the merged MVC, Web API, and Web Pages w/ Razor. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
5.62k stars 2.14k forks source link

Discussion: Simplifying MVC Link Generation - Guidance for Routing with Areas #3665

Closed rynowak closed 7 years ago

rynowak commented 8 years ago

Discussion thread for: aspnet/Announcements#120


Simplifying MVC Link Generation - Guidance for Routing with Areas

We're making a change in RC2 to remove a feature from Routing/MVC that I like to call magic link generation. This was added early-on as a companion to some changes in URL matching for MVC 6 with the hope that link generation would do the right thing in more cases.

In actuality magic link generation has garnered a "you moved my cheese" reaction more frequently than it has received praise. We're going to remove it and restore the MVC5 behavior for link generation.

Magic link generation only affects conventional routing (attribute routing doesn't need this feature) and only in cases where the action being linked to doesn't exist.

Description

Magic link generation prevents a route from generating a link, when the URL that would be generated doesn't match anything.

In MVC 6 beta-RC1 For instance, a route to the blog area /blog/{controller}/{action}/{id?} with values new { controller = "Products", "Index" } won't generate a link, unless there is a ProductsController with an Index() action in the blog area.

In MVC 6 RC2 & MVC1-5 The above example will generate the URL /blog/Products/Index without any consideration for whether or not this URL could be matched by a route.

Impact

Removal of this feature should only really affect your applications if you're defining routes to areas in a way that the less-specific (for link generation) routes come first. If you're using areas, update your routing definitions to look like one of the examples in the next section.

We'll be removing RoutingOptions.UseBestEffortLinkGeneration in a follow-up change. This was an opt-out for magic link generation behavior. If you're currently using RoutingOptions.UseBestEffortLinkGeneration == true then this change will likely have no impact on your application.

Guidance for using areas with conventional routes

If you're using conventional routing with areas, we recommend a few supported ways of authoring routes.

Example 1 - using {area:exists} in the template:

app.UseMvc(routes =>
{
    routes.MapRoute("area_route", "{area:exists}/{controller}/{action}/{id?}");
    routes.MapRoute("default_route", "{controller}/{action}/{id?}");
}

New in MVC6, you can use {area:...} as a token in route templates if url space is uniform across all areas. This is probably the least complicated option.

The only consideration is to put the route with {area:...} first. The reason why is this case:

@Url.Action("Index", "Home", new { area = "Admin" }); // Generates /Admin/Home/Index

The only thing stopping the {controller}/{action}/{id?} route from generating a link is ordering. Otherwise it would generate /Home/Index?area=Admin, which is not what you want.


Example 2 - individual routes for each area:

app.UseMvc(routes =>
{
    routes.MapAreaRoute("users_mgmt_route", "users", "Admin/Users/{controller}/{action}/{id?}");
    routes.MapAreaRoute("blog_route", "blog", "Blog/{controller}/{action}/{id?}");
    ...
    routes.MapRoute("default_route", "{controller}/{action}/{id?}");
}

This should look more familiar to how things worked in MVC5. The new MapAreaRoute extension method adds the provided area name (users, blog) as both a constraint and default to the routes, ensuring that they only generate a link when asked to.

Again, the routes that handle areas should come first a non-area route has the potential to generate a link that would be incorrect when trying to link to an action in an area.


The History

Read on only if you want an MVC history lesson

The problem that magic link generation solves is generating links to areas - there's no ordering of these routes that produces the correct link generation behavior. Unfortunately these routes are what an existing MVC5 user might have in their site.

Example 1:

app.UseMvc(routes =>
{
    routes.MapRoute("Admin/{controller}/{action}/{id?}", defaults: new { area = "Admin" });
    routes.MapRoute("{controller}/{action}/{id?}");
}

@Url.Action("Index", "Home"); // Generates /Admin/Home/Index
@Url.Action("Index", "Home", new { area = "Admin" }); // Generates /Admin/Home/Index

Example 2:

app.UseMvc(routes =>
{
    routes.MapRoute("{controller}/{action}/{id?}");
    routes.MapRoute("Admin/{controller}/{action}/{id?}", defaults: new { area = "Admin" });
}

@Url.Action("Index", "Home"); // Generates /Home/Index
@Url.Action("Index", "Home", new { area = "Admin" }); // Generates /Home/Index?area=Admin

In either of these cases, this is not desirable.. You can't use ordering to resolve the problem, because there's no ordering of routes that puts the 'most specific' first.

The lessons of history

MVC2-5 solved this problem by manually filtering the route collection. MVC2-5's link generation code had special knowledge of the area token (like it does controller and action), and would create a new route collection for each link generation operation containing just the routes that match.

While semantically correct, this solution was unacceptable from a performance point-of-view. Large sites like Orchard had to implement complicated workarounds to avoid this code path running. Since the route collection was part of System.Web and could change at any time (it was thread-safe) there wasn't much we could do to mitigate this problem.

Additionally, this solution has too much coupling to the concept of 'area' at multiple levels of the system. Routes in ASP.NET 5 are not introspectable like they were in ASP.NET 4, so we couldn't write this code even if we wanted to. In MVC6 we want to support arbitrary hierarchies of action-selection structure (like subareas) so we needed to find a way to support this behavior without hardcoding area into the link generation code.

Arriving at the MVC6 solution

We whiteboarded a lot of examples of using routing for areas and for 'static' routes directly to a controller or action. In the end what made sense was to rely on existing routing features rather than add something new. Routing is already very complex.

The new MapAreaRoute is implemented very simply to add a route with a default and constraint so that it only generates a URL when an area value is provided.

    routes.MapRoute(
        "admin_Route",
        "Admin/{controller}/{action}/{id?}",
        defaults: new { area = "Admin" },
        constraints: new { area = "Admin" });
Muchiachio commented 8 years ago

Nice write-up @rynowak, loving the history part :+1: Does this directly or indirectly address cases like #3340?

rynowak commented 8 years ago

This is not going to change the behavior of #3340. The issue at hand in that case is this:

MVC v2-5's link generation code had special knowledge of the area token (like it does controller and action)

In MVC v2-5, the URL generation code has special knowledge of the tokens area, controller and action. For every link that gets generated, the URL helper takes the current value of these three tokens and inserts it into the provided values. This has the effect of making these tokens not subject to route-value-invalidation.

Basically in MVC5, when you write Url.Action("Checkout") from inside the ProductsController in the Store area, it's as-if you wrote Url.Action("Checkout", "Products", new { area = "Store" }).

guardrex commented 8 years ago

@rynowak Thanks again for providing the context and explanation. Getting a 404 makes more sense to me than broken markup. :+1:

guardrex commented 8 years ago

@rynowak I missed something ...

This was my markup (RC1) ...

<form asp-antiforgery="true" asp-action="contact" method="post">

... which was rendering this ...

<form method="post" action="/contact">

I moved it to RC2, and now it's putting the controller in the action ...

<form method="post" action="/Contact/contact">

... which would ordinarily be fine. I guess what I'm asking is how to stop Razor from doing that in this case. I do have the controller for this view in a different file ContactController.cs, and the view is in Views > Contact > index.cshtml, but I want my routes throughout the app (in this case) to just hang off the apex domain (i.e., http://www.myapp.com/contact in this case).

rynowak commented 8 years ago

@GuardRex - post your routes?

My guess is that your routing configuration always wanted to do this, but magic was stopping it. My advice if you want to have full control over how links are generated is to use attribute routing and/or named routes.

guardrex commented 8 years ago

@rynowak

HomeController.cs

[HttpGet("/{v}/{y}")]
public IActionResult Error404() => RedirectToAction( ... );

[HttpGet("/{v?}")]
public IActionResult Index(string v)

ErrorController.cs

[HttpGet("/error/{v?}/{y?}")]
public IActionResult Index(string v, string y)

ProjectsController.cs

[HttpGet("/projects/{v?}")]
public IActionResult Index(string v)

ContactsController.cs

[HttpGet("/contact/{d?}")]
public IActionResult Index(string d, Visitor visitor = null)

[HttpPost("/contact/{d?}")]
public async Task<IActionResult> Index(Visitor visitor)

... and the only route that's wrong AFAIK is the <form> asp-action attribute in the index.cshtml of Views > Contact. Although, I think setting it like this ...

<form id="contactform" asp-antiforgery="true" asp-action="index" method="post">

... in the source markup is working ok, and perhaps I was supposed to be doing it that way anyway.

rynowak commented 8 years ago

Do you have any conventional routes defined? (in Startup.cs)

You indeed should be using <form id="contactform" asp-antiforgery="true" asp-action="index" method="post"> contact isn't the action name anywhere that I can see.

Think of it this way... all the 'ambient values' of the current request are combined with the values you specify to make a combined set. That set is then used to pick an action. That action's attribute route is then used to generate a URL.

Ex:

ambient values: new { controller = "Contacts", action = "Index" }
values (from taghelper): new { controller = "Contacts" action = "contact" }

combined values: new { controller = "Contacts" action = "contact" }

The combined values are what's used by routing to select the action used for link generation.

In MVC the action and controller are always specified (whether you provide them or not) when using Url.Action or Html.ActionLink or asp-action, etc...

guardrex commented 8 years ago

@rynowak Got it. My apps are routing well now. Thanks for explaining. I think what's going to help a lot is when the concepts for MVC Razor routing are put out into the Controllers docs. I'm looking forward to those sections.

dneimke commented 8 years ago

@rynowak any chance of pointing me at some guidance for writing tests against routing? It would make it easier to submit issues if I know how to write tests against cases :-)

Eilon commented 7 years ago

We are closing this issue because no further action is planned for this issue. If you still have any issues or questions, please log a new issue with any additional details that you have.