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

Url generator in view ignored Area Name when a another controller has AttributeRoute #7046

Closed
John0King opened this issue Nov 11, 2017 · 14 comments
Labels

Comments

@John0King
Copy link

MVC version

AspNetCore MVC 2.0.0 on .net Framework 4.6.1

description

Areas/
    Admin/
        Controllers/
            [Area("Admin")]NewsController.Index  ---  with Route.MapRoute("{area:exists}/{controller=Home}/{action=Index}/{id?}")

Controllers/
    NewsController.Index  --with AttributeRoute [Route("News")]

when I inside Admin area with <a asp-action="Index" asp-controller="News">link</> , It generate /News for me,
and if I remove the attributeRoute or specific asp-area="Admin" it will generate the right url /Admin/News

I don't know why the ambient values are ignored

@John0King John0King changed the title Url generator in view ignored Area Name when a other controller with AttributeRoute Url generator in view ignored Area Name when a another controller has AttributeRoute Nov 11, 2017
@Eilon
Copy link
Contributor

Eilon commented Nov 28, 2017

@John0King can you upload a complete repro to GitHub so we can take a look? I'm not sure there's enough information here for us to reproduce this.

@John0King
Copy link
Author

John0King commented Dec 1, 2017

@Eilon This is my sample repo , just run it and you can see the different url being generated for News and News2 controller when you insde admin area

@Eilon
Copy link
Contributor

Eilon commented Dec 1, 2017

@John0King thanks for sharing the sample.

@jbagga can you take a look?

@jbagga
Copy link
Contributor

jbagga commented Dec 2, 2017

@John0King

Actions that define attribute routes cannot be reached through the conventional routes

See https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/areas.

Attribute routing takes precedence over the conventional "{area}/{controller}/{action}" route you defined in your Startup (when you map a route for Areas), and you see the route you defined in RouteAttribute (which is just "News"). Removing the Route attribute should indeed work (that is expected).

If this isn't what your issue is, please let me know and I am happy to investigate further!

@John0King
Copy link
Author

@jbagga
In the document:

Generating links from an action within an area based controller to another action within the same controller.
Let's say the current request's path is like /Products/Home/Create
HtmlHelper syntax: @Html.ActionLink("Go to Product's Home Page", "Index")
TagHelper syntax: Go to Product's Home Page
Note that we need not supply the 'area' and 'controller' values here as they are already available in the context of the current request. These kind of values are called ambient values.

If like you said :

Attribute routing takes precedence over the conventional "{area}/{controller}/{action}" route

why the Url generate correctly when I specific asp-area="Admin" ?

and if this is a normal behaviour then in the document you should told people always specific the area value , because it's too dangerous (your current url may change because somewhere use attribute route)

@jbagga
Copy link
Contributor

jbagga commented Dec 5, 2017

The ambient values are used to fill in the blanks when you have a conventional route that could be a match. The example you quoted assumes a conventional route only.
The route parameters are not filled in by ambient values for an attribute route. https://docs.microsoft.com/en-us/aspnet/core/mvc/controllers/routing#attribute-routing

Specifying area="Admin" in values for Url.Action adds the area value to the area route parameter in the generated route. Your attribute route of "News" therefore does not match anymore and the next routes in queue (the conventional routes you specified in the Startup) are tried in order. The route pattern/template of "{area}/{controller}/{action}" then matches.

If you want precise control of how your controllers are routed to, use attribute routing for your controllers throughout the application. For instance add [Route("Admin/News")] to your Admin News controller and it will generate "Admin/News".

cc @pranavkm

@John0King
Copy link
Author

Now I understand why ambient values are ignored . but I don't like it :( .

Logically , URL should point to current area if possible .

image
I add more things to show in my test repo, If i'm correct, the RouteData.Values
are ambient values , and ActionDescriptor.RouteValues are used for MVC to find the Controller Action .

So when ambient values are ignored, the url generator does not pick up the values from ActionDescriptor.RouteValues ? I think it should be. but I'm not very sure how UrlHelper build the VirtualPathContext .

My Personal opinions:

  1. For at least 95% of user case , when you try to point to an empty area inside an area , you should specify asp-area="".
  2. And should be 100% of user case, use current controller and/or action when you not specify controller and/or actoin

It's reasonable to fallback to clone an ActionDescriptor.RouteValues and replace those value when ambient values are ignored.

I'll try to read the code and see how UrlHelper work.

@John0King
Copy link
Author

@jbagga

var valuesDictionary = GetValuesDictionary(actionContext.Values);

and
_routeValueDictionary = new RouteValueDictionary();

there no use of ActionDescriptor.RouteValues , should it use that ? and the code in UrlHeper.cs can be optimized with the if(a is A b) syntax

and because you share the _routeValueDictionary , does that mean if I create a parallel Task to generate url , the GetValueDictionary will be unreliable ?

@mkArtakMSFT
Copy link
Contributor

@John0King, to answer your last question: the code you referred to is not thread safe, so you may run into weird issues if you try to call the GetValueDictionary() from another thread.

@John0King
Copy link
Author

May I Ask why there need RouteData.Values instead of ActionDescriptor.RouteValues ?
I always have trouble when in [controller]/[action]/{Classify}/{SubClassify} and try to goto [controller]/[action]/{Classify} , the UrlHelper generate the same url when I not specify SubClassify as null

@John0King
Copy link
Author

@jbagga

when I inside Admin area with <a asp-action="Index" asp-controller="News">link</a> , It generate /News for me,

Will this behavior change in asp.net core 2.1 ?

I don't use atribute-route now (but I want to), because my admin area have many link looks like empty area

@pranavkm
Copy link
Contributor

pranavkm commented May 9, 2018

Will this behavior change in asp.net core 2.1 ?

This behavior will not change. It's by design, attribute routes are preferred to conventional routes.

why the Url generate correctly when I specific asp-area="Admin" ?

Because it adds an additional constraint that the attribute route does not meet.

The guidance here would be to consistently use attribute routing or conventional routing. Conflating the two, in particular, where your route values appear identical is going to result in the sort of issues you're running in to.

@John0King
Copy link
Author

But if we keep ActionContext.ActionDescriptor.RouteValues , we can avoid this issue .
In fact I prefer this behavior :

  • Url.Action() without parameters use ActionContext.RouteData.Values to generate current Url.
  • Url.Action(action:"Index"...) with parameters use ActionContext.ActionDescriptor.RouteValues to generate new Url.

unless if u in a route url [controller]/[action]/{v1}/{v2} for example Home/Index/Abc/1 , and u want to create a new url only change v2 , you use Url.Action("Home",new { v2=2 }) insteal of Url.Action("Home",new {v1="Abc",v2=2}) .

@aspnet-hello
Copy link

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.

@aspnet-hello aspnet-hello removed this from the Discussions milestone Sep 24, 2018
@aspnet aspnet locked and limited conversation to collaborators Sep 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants