Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

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

Closed
rynowak opened this issue Nov 30, 2015 · 10 comments
Closed
Milestone

Comments

@rynowak
Copy link
Member

rynowak commented Nov 30, 2015

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
Copy link
Contributor

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

@rynowak
Copy link
Member Author

rynowak commented Dec 2, 2015

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
Copy link

guardrex commented Dec 3, 2015

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

@guardrex
Copy link

guardrex commented Dec 5, 2015

@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
Copy link
Member Author

rynowak commented Dec 7, 2015

@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
Copy link

guardrex commented Dec 7, 2015

@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
Copy link
Member Author

rynowak commented Dec 7, 2015

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
Copy link

guardrex commented Dec 8, 2015

@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
Copy link

dneimke commented Dec 10, 2015

@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
Copy link
Contributor

Eilon commented Jun 9, 2017

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.

@Eilon Eilon closed this as completed Jun 9, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants